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

Fix energy #31

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix energy #31

wants to merge 1 commit into from

Conversation

clementhelsens
Copy link
Contributor

BEGINRELEASENOTES

  • Delphes mass are 0, but the energy is taken from the delphes candidate, without correcting for the non zero mass
    ENDRELEASENOTES

@tmadlener
Copy link
Contributor

Hi @clementhelsens, thanks for this.

There is one thing that we have to consider here. The tests currently break here:

compareCollectionElements<Track>(tracks, recoColl, "EFlowTrack", 0, mcRecoAssocColl);

This is because in the tests the 4-vectors of delphes candidates to the converted edm4hep candidates are compared directly, i.e. without adjusting for a potentially different mass in the energy. We had this discussion already some time ago, where we thought about making the 4 momentum internally consistent in edm4hep, but then decided to keep the momentum, mass and energy separate. I don't know what would be the best solution in this case, but we would need to at least update the tests accordingly if we keep this version.

In any case, I have added some basic utilities for 4 momentum vectors here: key4hep/EDM4hep-utils#1.

@clementhelsens
Copy link
Contributor Author

Hi @tmadlener , not sure what is the best solution either. I made this PR because I found this while doing analysis downstream (where I am correcting anyway).
Maybe needs to be discussed at one of the next meeting

@andresailer
Copy link

  • setting charged particle mass to pion mass
    * adjust identified muon and electron mass ?
    • nothing done for neutrals so far
      • K0L or n ?
    • PR: set the energy according to the mass hypotheses
      • will temporarily break the consistency check with pure Delphes output
      • Delphes authors interested in following this approach
    • should merge this PR now

@tmadlener
Copy link
Contributor

It looks like this needs a rebase to resolve the conflict.

Additionally, if I understand the comment above correctly a second setEnergy call would be necessary for the case where the mass is adapted for electrons and muons, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants