Port Pandora to Gaudi#16
Conversation
jmcarcell
force-pushed
the
port-pandora
branch
2 times, most recently
from
59f2644 to
8182725
Compare
jmcarcell
force-pushed
the
port-pandora
branch
from
b393dce to
a982787
Compare
tmadlener
left a comment
tmadlener
left a comment
There was a problem hiding this comment.
I am not sure whether we want to open that can of worms, but are we married to the naming here? I.e. do we want to e.g. keep the DD prefix? I am afraid that will become quite confusing at least for some time where both the Marlin and the Gaudi version are around simultaneously.
jmcarcell
force-pushed
the
port-pandora
branch
from
d6a6108 to
70f6e42
Compare
|
I think this is ready to be merged. The parts that are not implemented are the ones that create Track to MCParticle relations and CaloHit to MCParticle relations in the DDMCParticleCreator but these do not have any effect on the output and I don't see any parameter where this could be changed.
I don't have a strong opinion about the name, it's easy however to match files when they are called the same and I don't think there are many situations when they can be confusing, they are also in different organizations so you wouldn't find the two at the same if you search on Github. |
|
One comment on the naming of the components and ingredients here. IF we want to run this in combination with other Marlin processors via the wrapper, it's possible to have symbol (name) collisions, because both Gaudi and Marlin will load libraries dynamically and if they have the same name things will start to crash in weird ways. I am not entirely sure if this applies only to the algorithms / processors (that is where I have observed this), or also to the library parts that are used. I think the algorithm here is OK, since the original Marlin processor has |
jmcarcell
force-pushed
the
port-pandora
branch
from
21afcfa to
76f1a7c
Compare
streamlog statements commented until changed to use gaudi logging cellIDEncodings still need to be fixed as well.
needs fixing for streamlog, maybe properreview of what it does added ClusterShapes from MarlinUtil and made few minor changes to its interface for EDM4hep compatibility
I have very strong doubts it won't horribly crash, but it compiles streamlog commented again
… vertex collections there was no need to use the vertex collections just to get associated particle use void* as in Samf25's PR to store information about track relations
|
Merging today and tagging after. |
5 participants
This is an almost finished version of DDPandoraPFANewProcessor, taken from #11 and #12 with validation to check that the result is the same as the original processor.
Things to do:
DDPandoraPFANewAlgorithm(create the TrackToMCParticle relationships and also CaloHitToMCParticle relationships in Pandora) -> this is not done after not being able to use these relationships in the CLD reconstruction (i.e. creating them doesn't make any difference in the output with respect to not creating them)Related PR in CLDConfig: key4hep/CLDConfig#70