Skip to content
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

Fixing conversion of ScalarHT and make it a UserDataCollection #138

Merged
merged 4 commits into from
Dec 20, 2024

Conversation

bistapf
Copy link
Contributor

@bistapf bistapf commented Nov 20, 2024

BEGINRELEASENOTES

  • Use a UserDataCollection<float> to store the ScalarHT output from Delphes instead of a ParticleIDCollection.
  • Fix a typo in the converter source code that prevented the ScalarHT collection from Delphes from being converted, even if explicitly requested in the output config.

ENDRELEASENOTES

  • WIP, because what is the reasoning behind the ScalarHT collection being a ParticleIDCollection? This comes with a bunch of unneccessary, confusing parameters that don't apply (PDG, likelihood, algorithmType, .. ). Should we change this to e.g. UserDataCollection or how it's done for MissingET?

@tmadlener
Copy link
Contributor

Just to confirm with @kjvbrt and @BrieucF: I don't think ScalarHT is used anywhere at the moment, because it's mainly a hadron collider variable. So in that regard changing this to something more appropriate should not really be a problem.

@kjvbrt
Copy link
Contributor

kjvbrt commented Nov 21, 2024

Hi @tmadlener, searching through our repositories, there is this one card delphes_card_IDEAtrkCov.tcl#L58

There are also some other cards in EventProducer's spring2021 campaign, but I think they can be ignored by now.

I'm OK with changing it to something more meaningful.

@bistapf
Copy link
Contributor Author

bistapf commented Dec 20, 2024

Thanks @kjvbrt , @tmadlener . We have decided it makes most sense for ScalarHT to be a UserDataCollection, meaning only one variable/flat branch is stored for it in the output files.

Copy link
Contributor

@tmadlener tmadlener left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also update the documentation accordingly? Most importantly the table here https://github.com/key4hep/k4SimDelphes/blob/main/doc/output_config.md#class-conversions

converter/src/DelphesEDM4HepConverter.cc Show resolved Hide resolved
@bistapf bistapf marked this pull request as ready for review December 20, 2024 11:02
doc/output_config.md Outdated Show resolved Hide resolved
@tmadlener tmadlener changed the title Fixing conversion of ScalarHT Fixing conversion of ScalarHT and make it a UserDataCollection Dec 20, 2024
@tmadlener tmadlener enabled auto-merge (squash) December 20, 2024 12:12
@tmadlener tmadlener merged commit 2261a45 into key4hep:main Dec 20, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants