Add GGTF algorithm configuration for IDEA and ALLEGRO #315
Conversation
|
Thanks Stefano! I'll let some time for @giovannimarchiori to take a look, potentially will be merged after the break but I don't think it is super urgent, right? |
| "SimTrackHitCollectionName": ["VertexBarrelCollection"], | ||
| "SimTrkHitRelCollection": ["VTXBSimDigiLinks"], | ||
| "SubDetectorName": "VertexBarrel", | ||
| "TrackerHitCollectionName": ["VTXBDigis"], |
There was a problem hiding this comment.
Do you need SimTrackHitCollectionName, SimTrkHitRelCollection, TrackerHitCollectionName to be lists instead of strings? if they are lists, maybe better add "s" at the end e.g. "Names" , "Collections", ..
And later, adapt the code accordingly (I see that in L337 for instance you treat SimTrackHitCollectionName as a scalar...
There was a problem hiding this comment.
I agree that this one is a bit awkward.
These dictionaries represent the key-value pairs that are used to schedule the DDPlanarDigi algorithm. The latter requires input lists, despite the attributes are named *CollectionName. Passing strings directly, I'm getting the following error:
ValueError: received an instance of <class 'str'>, but <class 'list'> expected for property SimTrackHitCollectionName
Since we are making these dictionaries, I thought it would be worth using them also for the legacy digitizers, for ensuring naming consistency. But as you point out in your comment below, then we need to explicitly pick the list item and cannot pass the list directly to the VTXdigitizer (using strings I get a similar error as above, but complaining about lists arguments that should be str instead).
Even if it's a bit dirty, I think it can be worth keeping this options' mapping for the new digitizers to the legacy ones. But if you prefer to drop it, I can remove it and add back the entries as they were before.
| "SimTrackHitCollectionName": ["VertexEndcapCollection"], | ||
| "SimTrkHitRelCollection": ["VTXDSimDigiLinks"], | ||
| "SubDetectorName": "VertexDisks", | ||
| "TrackerHitCollectionName": ["VTXDDigis"], |
| "SimTrackHitCollectionName": ["SiWrBCollection"], | ||
| "SimTrkHitRelCollection": ["SiWrBSimDigiLinks"], | ||
| "SubDetectorName": "SiWrB", | ||
| "TrackerHitCollectionName": ["SiWrBDigis"], |
| "SimTrackHitCollectionName": ["SiWrDCollection"], | ||
| "SimTrkHitRelCollection": ["SiWrDSimDigiLinks"], | ||
| "SubDetectorName": "SiWrD", | ||
| "TrackerHitCollectionName": ["SiWrDDigis"], |
| inputSimHits=vxd_barrel_digi_args["SimTrackHitCollectionName"][0], | ||
| outputDigiHits=vxd_barrel_digi_args["TrackerHitCollectionName"][0], |
There was a problem hiding this comment.
if SimTrackHitCollectionName and TrackerHitCollectionName are lists, shouldn't you use vxd_barrel_digi_args["SimTrackHitCollectionName"][0] and so on?
But probably better make them scalars before...
|
Hi @sfranchel , |
Co-authored-by: Giovanni Marchiori <[email protected]>
Ciao @giovannimarchiori, thanks a lot for the review! Your inline suggestions are all merged, and I've replied more in detail to your comment above. Let me know how you want to proceed on that one! |
3 participants
Schedule the Geometric Graph Track Finding algorithm for the IDEA_o1_v03 and ALLEGRO_o1_v03 reconstructions.
DDPlanarDigiAdditionally, adds option in ALLEGRO script to specify the path where the files needed for digitization and reco steps are. In this way, if eos access is available, there's no need to copy files around.