CROWN Changes for mumu to ee embedding - #372
Conversation
…et corrections in the same way '
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| CodeStyle | 1 minor |
🟢 Metrics 6 complexity · 0 duplication
Metric Results Complexity 6 Duplication 0
TIP This summary will be updated as you push new changes. Give us feedback
There was a problem hiding this comment.
Pull request overview
This PR adds new event-level helpers to support embedding workflows (muμ → ee), and extends code generation configuration with additional sample-type flags plus a safer config cleanup loop.
Changes:
- Added embedding-muon charge matching utilities (
MatchEmbeddingMuonP4ByCharge,HasEmbeddingMuonChargeMatch) and an opposite-charge helper (HasOppositeCharge). - Extended
Configuration._set_sample_parameters()with explicitis_data,is_dyjets, andis_wjetsboolean flags; made_remove_empty_configkeys()safe against mutation during iteration. - Added
.codexto.gitignore.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/event.cxx | Implements new charge-based embedding-muon matching helpers and opposite-charge flag helper. |
| include/event.hxx | Exposes declarations for the new event::quantity helpers. |
| code_generation/configuration.py | Adds extra sample flags and fixes dict-key iteration during deletion. |
| .gitignore | Ignores .codex artifacts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #include "../include/utility/Logger.hxx" | ||
| #include "ROOT/RDataFrame.hxx" | ||
| #include "TRandom3.h" | ||
| #include <cmath> |
| auto has_match = [](const int &reco_q, | ||
| const float &emb_q_1, | ||
| const float &emb_q_2) { | ||
| return static_cast<int>(reco_q == static_cast<int>(emb_q_1) || | ||
| reco_q == static_cast<int>(emb_q_2)); | ||
| }; |
| sample_parameters["is_data"] = False | ||
| if "dyjets" in self.sample: | ||
| sample_parameters["is_dyjets"] = True | ||
| else: |
| if "data" in self.sample: | ||
| sample_parameters["is_data"] = True | ||
| else: | ||
| sample_parameters["is_data"] = False | ||
| if "dyjets" in self.sample: | ||
| sample_parameters["is_dyjets"] = True | ||
| else: | ||
| sample_parameters["is_dyjets"] = False | ||
| if "wjets" in self.sample: | ||
| sample_parameters["is_wjets"] = True | ||
| else: | ||
| sample_parameters["is_wjets"] = False |
There was a problem hiding this comment.
Why is this needed? This is already done by the if else above.
| None | ||
| """ | ||
| for key in config: | ||
| for key in list(config.keys()): |
There was a problem hiding this comment.
Was this breaking anything?
There was a problem hiding this comment.
Agree. I would keep the old version.
| return static_cast<int>(reco_q == static_cast<int>(emb_q_1) || | ||
| reco_q == static_cast<int>(emb_q_2)); |
There was a problem hiding this comment.
Agree. The embedding selection already imposes opposite-sign charge muons, so these conditions should always be fulfilled. However, the function could be useful for consistency checks.
| const std::string &outputname, | ||
| const std::string &q_1, | ||
| const std::string &q_2) { | ||
| auto has_opposite_charge = [](const int &q_1, const int &q_2) { |
There was a problem hiding this comment.
Why are here the charges integer but in the other functions floats?
|
Is this PR still worked on? |
| None | ||
| """ | ||
| for key in config: | ||
| for key in list(config.keys()): |
There was a problem hiding this comment.
Agree. I would keep the old version.
| const UInt_t &master_seed = 42 | ||
| ); | ||
|
|
||
| // Select the embedding-muon p4 that has the same charge as the reco object. |
There was a problem hiding this comment.
Please run clang-format (https://github.com/KIT-CMS/CROWN/blob/main/checks/cpp-formatting.sh). Merging the branch will be blocked because the code formatting test is not going to pass.
| * | ||
| * @return a dataframe with the new Lorentz vector column | ||
| */ | ||
| ROOT::RDF::RNode MatchEmbeddingMuonP4ByCharge( |
There was a problem hiding this comment.
The name of the function suggests that a matching takes place and that a boolean value or the matching metric is returned. A name like GetEmbeddingMuonP4ByCharge suits better here.
| return static_cast<int>(reco_q == static_cast<int>(emb_q_1) || | ||
| reco_q == static_cast<int>(emb_q_2)); |
There was a problem hiding this comment.
Agree. The embedding selection already imposes opposite-sign charge muons, so these conditions should always be fulfilled. However, the function could be useful for consistency checks.
No description provided.