Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 additional track variables to the Run 3 scouting electron collection for low pT electrons. #41025
Add additional track variables to the Run 3 scouting electron collection for low pT electrons. #41025
Changes from all commits
754a5cd
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I'm confused why this
clear()
is needed (but I acknowledge the evidence #41025 (comment), #41025 (comment)). @pcanal Could you comment if from ROOT point of view thisclear()
serves a purpose, or if the need for one is a sign of something else going on?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.
Since
trkd0
is a 'new' data member, when reading back an old version on of the object, this data member will be untouched. So indeed theclear
is necessary to insure that the vector contains only new data. Note that if more than one rule want to put elements in the vector, theclear
should (of course) be called only in the 'first' rule to be executed.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.
Thanks @pcanal for the explanation. If the change would keep the member variable name the same instead (i.e.
float d0_
->std::vector<float> d0_
), the rule would be along (right?)then the
d0_.clear()
would not be needed?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.
I'm sure the
clear
would still be needed if one were using the 'recommended' way of reading back data which has one reusing the same object over and over. cmsRun doesn't do that and instead forcibly creates a new object for each event.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.
Actually yes (the in-memory version of
d0_
would also be untouched by the I/0 in that case).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.
Thanks. In the end it is probably cleaner to keep the io rule as it is.