Follow-up from "WIP:Adding module that creates a fixed passive material"
The following discussions from !213 (merged) should be addressed:
-
@simonspa started a discussion: Somehow the way this is currently done is a bit circular: you add a pointer to the GeometryManager here before adding the whole object to the GeometryManager itself. While I see the requirement for doing so in the current code, it doesn't feel very elegant to me.
-
@pschutze started a discussion: (+5 comments) Hi Koen,
I tried out your branch and something still seems to be problematic with the building of the geometry: When I run a full simulation chain, the configuration error pops up, even though I position the
PassiveMaterialBuilderGeant4
as a first module.Meaning, this works flawlessly (ignoring the parameters now):
[AllPix] [PassiveMaterialBuilderGeant4] [GeometryBuilderGeant4] [DepositionGeant4]
, but this won't:
[AllPix] [PassiveMaterialBuilderGeant4] [GeometryBuilderGeant4] [DepositionGeant4] [ElectricFieldReader]
Here I get
(FATAL) [I:PassiveMaterialBuilderGeant4] Error during execution of run: Geometry is already closed! Move this module above GeometryBuilderGeant4 in the configuration file Please check your configuration and modules. Cannot continue.
Can you reproduce this behaviour?
-
@kvandenb started a discussion: (+5 comments) I think I've addressed most of the comments of Simon in this merge-request. It should also fix the problem that @pschutze had with the ElectricFieldReader. I've cleaned up the core, merged the modules, added the "_log" suffix by code and let the PassiveMaterial file be read inside the GeometryBuilderGeant4Module instead of globally.
I'm not sure how easy it is to get information from inside the material. If a particle hits the sensor the whole McTrack information is called, which could be traced back to the position of the passive material. I'm not entirely sure, but I think each interaction creates a new particle from the point of interaction (a particle scattering of a target would create 2 new particles, one of the target and one of the scattered particle from the beam). This would mean, for all particles that hit a sensor, that if the begin of their track originates inside the passive material, this would mean it interacted in there. This only works for particles that hit a sensor and will be measured, for all other particles the McTrack does not get saved. Is this what you had in mind @simonspa?
A few more things.
-
Could you explain the change in GetExternalObject you suggested so I can try to fix it? You want two seperate ExternalObject objects, one specific GetExternalDetectorElement only for detectors, one GetExternalObjectElement for all elements including the detector elements?
-
I'm calling the mother volume now using the G4LogicalVolumeStore, but this could also be done using the GetExternalObject function if preferred. This could also work for the CheckOverlap function in the G4GeometryConstruction, reducing the dependency on Geant4.
-
I still have to change the Readme.md and the UserManual, that'll be done tomorrow
-
-
@simonspa started a discussion: (+1 comment) I believe that we had the code here to not have to store the PRNG - storing this will only lead to inconsistencies and problems in the future when people start using this elsewhere.
Is there a reason you moved this to a function apart from looking a bit more clean?
-
@simonspa started a discussion: (+15 comments) If you decide to keep this for whatever reason it should be made
private
- currently you are re-calculating the position and orientation every time this (public) function is called. Since we use random numbers for misalignment, this will lead to odd effects.(the name would then have to be
get_orientation
)