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 index to primary vertex in scouting particle collection #11033
Add index to primary vertex in scouting particle collection #11033
Conversation
A new Pull Request was created by @davidsheffield (David Sheffield) for CMSSW_7_4_X. Add index to primary vertex in scouting particle collection It involves the following packages: DataFormats/Scouting The following packages do not have a category, yet: DataFormats/Scouting @Martin-Grunewald, @perrotta, @cmsbuild, @fwyzard can you please review it and eventually sign? Thanks. |
please test |
The tests are being triggered in jenkins. |
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
+1 |
please test |
The tests are being triggered in jenkins. |
//default constructor | ||
ScoutingParticle():pt_(0), eta_(0), phi_(0), m_(0), pdgId_(0) {} | ||
ScoutingParticle():pt_(0), eta_(0), phi_(0), m_(0), pdgId_(0), vertex_(0) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given 0 is a valid index, how about using max int instead? That way reading old data one can spot if the variable is not filled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or -1 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Dr15Jones, you mean changing it to vertex_(2147483647)
? Or should I use a smaller value in case there maximum int is 32 bit?
-1 is a valid value for this member. The producer sets it to -1 when no vertex is matched.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given when reading old data there would be no match, then -1 makes the most sense.
I got the same checksum from scram b for the class as before the most recent commit despite changing the code. I assume that's wrong. How should I get the checksum? |
Pull request #11033 was updated. @Martin-Grunewald, @perrotta, @cmsbuild, @fwyzard can you please check and sign again. |
@davidsheffield -the checksum is derived from the member data not the c++ that fills them. So all is ok. |
@cmsbuild, please test |
The tests are being triggered in jenkins. |
comparison is not ready as jenkins job failed to upload the result to cmssdt server. I have restarted the comparison. |
…VertexAssociation Add index to primary vertex in scouting particle collection
Backport of #11029.