Skip to content

[PWGJE] Add configurable MC closure split and antiproton DCA templates - #18347

Merged
nzardosh merged 7 commits into
AliceO2Group:masterfrom
arvindkhuntia:master
Oct 20, 2026
Merged

nzardosh merged 7 commits into
AliceO2Group:masterfrom
arvindkhuntia:master

Conversation

@arvindkhuntia

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

O2 linter results: ❌ 0 errors, ⚠️ 60 warnings, 🔕 0 disabled

@arvindiiti1992

Copy link
Copy Markdown

Hi @nzardosh,
Greetings of the day.
Could you please approve the PR?

Many thanks.

Comment thread PWGJE/Tasks/nucleiInJets.cxx Outdated
// Event-wise random splitting for closure test: decide once per event
bool useDataLikeHist = (randUniform.Uniform(0, 1) < 0.5);
// One random choice is shared by the reconstructed and generated spectra of this event.
const bool useTrainingHist = randUniform.Rndm() < mcClosureTrainingFraction;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it ok that this random number selection is done independently in different process functions? Shouldnt it be done once per collision and that collision should always have the same status of being training or not in all process functions?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @nzardosh,
Thank you very much for the comment. I updated the splitting so the same MC collision receives the same training/data-like treatment in both process functions.

@@ -953,6 +983,34 @@ struct nucleiInJets {
template <bool IsMC, typename TracksType, typename JetType>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this IsMC is not used anywhere here I think? should you not gaurd against filling some blocks with if constexpr(!isMC) ?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for pointing this out. I added if constexpr (!IsMC) around the data-only template filling.

Comment thread PWGJE/Tasks/nucleiInJets.cxx Outdated
}

// Fill detector-level DCA templates before the DCAxy selection used by the spectra.
const double templateJetPt = mcdJetPt.at(indexJet) - (usebkgSubractionMC ? 0.f : backgroundRho * jetArea);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this correct? isnt the background subtraction the opposite? so it should be
usebkgSubractionMC ? backgroundRho * jetArea : 0.f

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The background subtraction is already applied when constructing mcdJetPt, so the extra correction here was not needed. I removed it and use mcdJetPt.at(indexJet) directly.

Comment thread PWGJE/Tasks/nucleiInJets.cxx Outdated
Service<o2::ccdb::BasicCCDBManager> ccdb;
Service<o2::framework::O2DatabasePDG> pdgDB;
TRandom3 randUniform;
UInt_t mcClosureSplitSeedEffective = 1;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if this is the correct type to use because the global indices might not be in this 32 bit format and might be 64 bit instead? you can check

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I checked this and changed the implementation to use uint64_t with std::mt19937_64, so the full 64-bit global index is preserved.

@nzardosh
nzardosh self-requested a review October 20, 2026 12:16
@nzardosh
nzardosh enabled auto-merge (squash) October 20, 2026 12:17
@nzardosh
nzardosh merged commit b313b31 into AliceO2Group:master Oct 20, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

4 participants