Support for PandoraPFA on ALLEGRO #31
Conversation
src/DDCaloHitCreatorALLEGRO.cc
Outdated
|
|
||
| // check if the hit in barrel | ||
| // FIXME! AD: ECal barrel systemID is hardcoded | ||
| if (cellIdDecoder(pCaloHit)["system"] == 4) |
There was a problem hiding this comment.
This is the only change in this function, apart from removing some other block you don't care about?
Can you rather use DDCaloHitCreaters function and add something like
//...
ifUseSystemId && cellIdDecoder(pCaloHit)["system"] == ecalSystemID
//...To the existing if statement?
src/DDCaloHitCreatorALLEGRO.cc
Outdated
|
|
||
| // check if the hit in barrel | ||
| // FIXME! AD: HCal barrel systemID is hardcoded here | ||
| if (cellIdDecoder(pCaloHit)["system"] == 8) |
There was a problem hiding this comment.
This is the only change in this function, apart from removing some other block you don't care about?
Can you rather use DDCaloHitCreaters function and add something like
//...
ifUseSystemId && cellIdDecoder(pCaloHit)["system"] == hcalSystemID
//...To the existing if statement?
src/DDCaloHitCreatorALLEGRO.cc
Outdated
|
|
||
| float absorberCorrection(1.); | ||
|
|
||
| if (isInBarrelRegion) |
There was a problem hiding this comment.
Can you just set m_settings.m_coilOuterR to 0 and use the existing function from DDCaloHitCreator? Or add a flag to disable the isWithinCoil flag?
| subDetectorTypeMap[pandora::COIL] = coilParameters; | ||
| */ | ||
|
|
||
| // FIXME! AD: below an ugly way is used to define the geometry of the Solenoid since we do not have the LayeredCalorimeterData for Coil in ALLEGRO |
There was a problem hiding this comment.
Why not add this to the coil?
There was a problem hiding this comment.
I am not sure if I understand your question.
Do you mean why we do not have CaloExtension for COIL?
There was a problem hiding this comment.
it's because I could not find how the coil/solenoid is built in ALLEGRO.
I guess it is currently not there.
Of course, this code will change in the future and caloExtension will be added.
There was a problem hiding this comment.
Yes, there is currently no actual coil, it is accounted for in the material of the outer radius side of the cryostat
| /** | ||
| * @brief DDTrackCreatorALLEGRO class | ||
| */ | ||
| class DDTrackCreatorALLEGRO : public DDTrackCreatorBase |
There was a problem hiding this comment.
Why not inherit from DDTrackCreatorCLIC and reduce the amount of copy-pasta?
There was a problem hiding this comment.
The functions re-used in the DDTrackCreatorALLEGRO are supposed to change for ALLEGRO. Currently only few things are disabled, but implementation of the (truth) tracks in the ALLEGRO is not finished yet.
Co-authored-by: Andre Sailer <andre.philippe.sailer@cern.ch>
|
Hi @andresailer , is there anything pending here or can we proceed? (I do not have the rights to trigger the tests) |
include/DDGeometryCreator.h
Outdated
| @@ -21,6 +21,9 @@ | |||
| class DDGeometryCreator | |||
| { | |||
| public: | |||
| // give access to the private memebers from DDGeometryCreatorALLEGRO | |||
| friend class DDGeometryCreatorALLEGRO; | |||
There was a problem hiding this comment.
Should rather make the private entries protected. And make some functions virtual when they are overridden in the inherited class.
There was a problem hiding this comment.
Done as suggested, for DDCaloHitCreator as well.
BEGINRELEASENOTES
ENDRELEASENOTES