feat(califa): Add (optional) new parameter ToT eff.#1334
feat(califa): Add (optional) new parameter ToT eff.#1334pgrusell wants to merge 1 commit intoR3BRootGroup:devfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates CALIFA simulation configuration to support floating-point thresholds and introduces an optional per-crystal ToT-efficiency parameter that probabilistically suppresses hits during digitization.
Changes:
- Change crystal simulation thresholds from
Int_t/TArrayItoFloat_t/TArrayF. - Add a per-crystal ToT-efficiency array (defaulting to 1 when unavailable) to the simulation parameter container.
- Apply ToT efficiency in
R3BCalifaDigitizer::FillRealConfig()via an accept/reject random draw.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
califa/sim/R3BCalifaDigitizer.cxx |
Applies the new per-crystal ToT efficiency during real-configuration digitization. |
califa/pars/R3BCalifaCrystalPars4Sim.h |
Adds accessor/member for ToT efficiency and switches threshold storage to float. |
califa/pars/R3BCalifaCrystalPars4Sim.cxx |
Allocates/stores/fetches the new ToT-efficiency parameter array and updates container metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fEffToTArray->Set(fNumCrystals); | ||
| list->add("califaEffToTPar", *fEffToTArray); | ||
|
|
There was a problem hiding this comment.
putParams() adds the ToT-efficiency array under key "califaEffToTPar", but getParams() tries to read "califaToTEffPar". As written, reading will always fail and the code will silently default all efficiencies to 1, so the new ToT-efficiency feature can never take effect. Please use a single consistent parameter name (and consider supporting the legacy name as a fallback if needed for backward compatibility).
feat(califa): Thresholds for simulations changed from Int_t to Float_t. feat(califa): Add ToT efficiency in the digitizer. fix(califa): Add copilot suggestions.
pgrusell
force-pushed
the
califa_tot_eff
branch
from
11e520c to
3874f14
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for (int i = 0; i < fNumCrystals; i++) | ||
| fEffToTArray->SetAt(1., i); | ||
| } | ||
|
|
There was a problem hiding this comment.
ToT efficiency is treated as a probability, but values read from the parameter container are not validated. If a value is <0 it will silently reject all hits for that crystal; if >1 it will accept everything. Consider clamping each entry to [0, 1] (and optionally logging when values are out of range) after loading califaToTEffPar so misconfigured parameter files don't produce surprising results.
| // Clamp ToT efficiencies to [0, 1] to avoid misconfigured parameters | |
| for (int i = 0; i < fNumCrystals; i++) | |
| { | |
| Float_t eff = fEffToTArray->At(i); | |
| if (eff < 0.f) | |
| { | |
| LOG(warn) << "califaToTEffPar value out of range for crystal " << i | |
| << ": " << eff << " (clamped to 0)"; | |
| fEffToTArray->SetAt(0.f, i); | |
| } | |
| else if (eff > 1.f) | |
| { | |
| LOG(warn) << "califaToTEffPar value out of range for crystal " << i | |
| << ": " << eff << " (clamped to 1)"; | |
| fEffToTArray->SetAt(1.f, i); | |
| } | |
| } |
feat(califa): Thresholds for simulations changed from Int_t to Float_t.
feat(califa): Add ToT efficiency in the digitizer.
Modifications in CALIFA simulations:
Checklist:
devbranch