refactor: use edm4eic::momenta() in vm_mass.cxx#96
Open
Conversation
…ch helpers Replace common_bench::momenta_RC and common_bench::momenta_from_simulation with the canonical edm4eic::momenta() overloads now available in edm4eic/analysis_utils.h. These are type-dispatched overloads taking ReconstructedParticleData or MCParticleData vectors respectively. Since momenta() is overloaded, use explicit lambdas in RDataFrame Define calls to select the correct overload. Also remove the now-unused #include "common_bench/mt.h" and the ROOT::EnableImplicitMT(kNumThreads) call is replaced with no-arg form (consistent with all other benchmark scripts). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the DVMP vm_mass analysis to use the canonical edm4eic::momenta() helpers (for both reconstructed and MC particles), aligning this script with the newer EDM4eic analysis utilities and removing reliance on common_bench momenta helpers and thread-count configuration.
Changes:
- Replace
common_bench::momenta_RC/common_bench::momenta_from_simulationwith explicit-lambda calls to the appropriateedm4eic::momenta()overloads in the RDataFrame flow. - Remove
common_bench/mt.husage by switchingROOT::EnableImplicitMT(kNumThreads)toROOT::EnableImplicitMT(). - Add required EDM4eic/EDM4hep includes for the new momenta helpers and MC particle types.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| #include "edm4eic/ReconstructedParticleCollection.h" | ||
| #include "edm4eic/ReconstructedParticleData.h" | ||
| #include "edm4hep/MCParticleCollection.h" |
There was a problem hiding this comment.
edm4hep/MCParticleCollection.h is included but this file only uses edm4hep::MCParticleData in the lambda. Consider including the corresponding Data header (or relying on an existing include) instead of the Collection header to reduce dependencies/compile time.
Suggested change
| #include "edm4hep/MCParticleCollection.h" | |
| #include "edm4hep/MCParticleData.h" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Replace
common_bench::momenta_RCandcommon_bench::momenta_from_simulationwith the canonicaledm4eic::momenta()overloads now available inedm4eic/analysis_utils.h.Since
momenta()is overloaded (for RC and MC particle types), explicit lambdas are used in the RDataFrame.Define()calls to select the correct overload.Also removes the
#include "common_bench/mt.h"(already removed in #95) and replacesROOT::EnableImplicitMT(kNumThreads)with the no-arg form (consistent with all other benchmark scripts).Depends on: eic/EDM4eic#... — requires the new
edm4eic::momenta()overloads to be available in the container.