-
Notifications
You must be signed in to change notification settings - Fork 127
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
feat: Schema for the oldstyle edm4hep Future Circular Collider simulation Samples #1182
Conversation
Hi! Sorry for taking some time to get back to you on this. This schema looks really well put together, thank you, and thanks for adding some basic tests. I'll get through a review soon. |
It's a bit less clean than yours but you may also want to have a look at #822. It's more a work in progress though. |
What would be everyone's opinion on generalizing this to any edm4hep? It should be relatively easy, mostly you have to deal with missing branches as in NanoAOD. |
@lgray Thanks for taking the time to review. #822 was indeed, a starting point for the development of FCCSchema. Many pre-generated samples in FCC Events, especially the Spring2021 and Winter2023, are Yeah, creating a similar schema for the FCCSamples based on new style edm4hep (eg. file generated by @davidlange6) must be fairly easy. Looks like #822 describes a general edm4hep schema, but the names of the branches assumed there, do not match with ones in FCC events. It would be really good if we could have a general edm4hep schema, and fcc schema could inherit from that, adding fcc specific functionality. Nevertheless, the current FCCSchema( oldstyle edm4hep ) needs to be there, to facilitate interpretation of the older campaigns (Spring2021, Winter2023, etc.) @davidlange6 What's your opinion on this? |
Oh those were some initial comments. I have more that I want to go through, but it'll be monday or so. |
right - its the eventual goal.. @prayagyadav and I thought to use this as an initial version to get feedback (and eventually users) on |
@tmadlener FYI this is was I was talking about yesterday. |
…ffea into review-fcc-schema
- Added Global Indexers for parents, daughters, Muons, Electrons, MCRecoassociations - Moved index_range function to transforms to avoid checking for typetracers etc.
Hi! @lgray , I figured out how to add global indexers and cross-references. I think I have fixed all of the issues that you had pointed out. Sorry for the unsquashed commits (I thought I did squash them, but evidently, it did not work). I think I will add some more tests later to check the validity of parents and daughters cross references. For now, I have this simple test in this notebook. |
@prayagyadav Thanks for following this up despite lack of feedback. I was busy at a workshop. What you have implemented looks correct. I'll wait for your additional tests and then we'll merge! |
Closes #822 |
for more information, see https://pre-commit.ci
Hi @lgray, I have added the tests for parents and daughters. I have also added one more sample file (needed to test the winter2023 campaign of FCC-Events). |
@prayagyadav For your 4th checkbox, do you want to address that in a later PR, or are you waiting for some more prompt feedback? |
I can address that in a later PR when I figure out that part of the functionality. |
@prayagyadav all looks good - auto-merge enabled on this latest CI run. Thanks a lot for your work! We should plan with @davidlange6 and @tmadlener about extending this to new-style edm4hep. I have made a tracking issue for the Tracks and Clusters functionality. We'll follow up in time. |
For completeness, and because it might be useful for development: This workflow of EDM4hep creates a dummy EDM4hep file that contains all available datatypes and relations between them. That should give you all the branch names (and actual c++ datatypes if that matters here) for EDM4hep. |
Awesome! |
Hi,
This is a schema for the Future Circular Collider Spring2021 pre-generated samples based on oldstyle EDM4HEP. This is a work in progress and I wanted to post it here for community inputs.
I have added most of the features:
Further development will need inputs from more people.
Thanks
Links and References:
p8_ee_ZH_ecm240
sample from here for testing (generated a smaller file with 100 events)Tagging my guides: @davidlange6 @gomber