-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add local Measurement{1,2,3}D classes #161
Comments
As mentioned during the hackathon already the main concerns from EDM4hep side are the following:
For the first one I have no clever solution yet, other than discouraging usage via documentation. The second one should be a bit easier to handle by providing some access functionality via some user / ACTS provided enum (or comparable). This would essentially work by mapping enum values to indices, as the access via integer indices will always be possible. I would have to actually sit down and try to come up with an implementation to see where this might fail. One thing that is not yet entirely clear to me is whether ACTS would also want to persist the A final thought about the actual implementation: Currently podio does not support building collections of "heterogeneous" types, i.e. it would not be easily possible to put measurements of different dimensions into a collection. Having different types for the different dimensions would be very much preferred for EDM4hep, mainly because an N dimensional "type erased" type, would make the first concern from above even more difficult to handle. |
Hey @tmadlener, thanks for the input! I agree with your first point. Clearly this should be well-defined. However, I do think a local measurement + geometry connection is a reasonably well-defined concept that it doesn't allow "user-data", especially if we tie the axis interpretation to an enum that lives in edm4hep. This doesn't allow "user-data" any more than say the current I'm still not sure I quite follow your 2nd point. With my proposal of adding a way to specify the local axes at the edm4hep level we break any dependency on ACTS, and the concept of a local measurement + axis definition is quite generic. Aside from this, there's no "proper" way for us to fill We could get away without a heterogeneous collection if another type can be set up to reference varying measurements. I.e. if a track state can have a variant pointing either at a 1D, 2D or 3D measurement (etc). If this isn't possible either, we could work around this by having a track state have multiple links, out of which only one is populated. If all of this fails, we'd have to go back to a static fixed size array that we only fill parts of and keep track of the dimension in another variable. On another note, and this is largely a strategy discussion, is that we only need these intermediate EDM types if the chain is supposed to be modular in the end. Ideally, we would want to allow On top of this, I think the Track/TrackState EDM will be a whole other discussion anyway. Like I said above, trackstates will have to link back to their local measurements to retain full information. If we don't do this, ACTS tracks can't be read in again as information has been discarded. |
I think here we probably have the same goal (and potentially even a very similar idea) in mind, but are just coming from two different directions. Maybe this is more easily solved and discussed on an actual example of a
Here we have had a sort of prototype some time ago, but were then sidetracked by other issues: AIDASoft/podio#215. The main issue with heterogeneous collections in podio is that storage really expects homogeneous collections, but this prototype would allow to have heterogeneous collections in memory at least. podio would do the heavy lifting of pulling in the references from the different collections that are actually stored. We would probably have to see how far we can take this approach. |
As discussed in the AIDAInnova hackathon during during CW25 2022, we should investigate providing classes for proper local measurements connected to a geometry description. The conclusion was to skip
TrackerHitPlane
in direct ACTS output, and provide aMeasurementXD
toTrackerHitPlane
conversion mechanism instead.Can we use this issue to discuss this further? @gaede @andresailer @vvolkl @andiwand @asalzburger @tmadlener
The text was updated successfully, but these errors were encountered: