light yield fix for Run-2 - #917
Conversation
PetrilloAtWork
left a comment
There was a problem hiding this comment.
I left some minor comments.
This is a configuration-breaking change, and I mentioned a way not to make it so.
However, this PR is very well written and very well documented.
| geo::Point_t const PMTcenter = applySd | ||
| ? fParams.wireReadoutGeom->OpDetGeoFromOpChannel(channel).GetCenter() | ||
| : geo::Point_t{}; |
There was a problem hiding this comment.
I would argue that, provided that wireReadoutGeom is available, this PMTcenter is better unconditionally set rather than leaving it with a potentially dangerous value { 0, 0, 0 }:
| geo::Point_t const PMTcenter = applySd | |
| ? fParams.wireReadoutGeom->OpDetGeoFromOpChannel(channel).GetCenter() | |
| : geo::Point_t{}; | |
| geo::Point_t const PMTcenter = fParams.wireReadoutGeom->OpDetGeoFromOpChannel(channel).GetCenter(); |
| ; | ||
|
|
||
| out << '\n' << indent << "Tail suppression: " | ||
| << std::boolalpha << fParams.tailSuppression.apply; |
There was a problem hiding this comment.
You can also merge this with the previous block:
| ; | |
| out << '\n' << indent << "Tail suppression: " | |
| << std::boolalpha << fParams.tailSuppression.apply; | |
| << '\n' << indent << "Tail suppression: " | |
| << std::boolalpha << fParams.tailSuppression.apply | |
| ; |
Just saying.
| // | ||
| fBaseConfig.distanceSurvival.apply = config.DistanceSurvival().Apply(); | ||
| fBaseConfig.distanceSurvival.binEdges = config.DistanceSurvival().BinEdges(); | ||
| fBaseConfig.distanceSurvival.factors = config.DistanceSurvival().Factors(); |
There was a problem hiding this comment.
As written, this PR is a configuration-breaking change.
Given that the configuration structures have sensible defaults, you may consider to make the new configuration tables optional (fhicl::OptionalTable()) and here you can go conditional:
| fBaseConfig.distanceSurvival.factors = config.DistanceSurvival().Factors(); | |
| // | |
| // tail suppression | |
| // | |
| if (config.TailSuppression()) { | |
| fBaseConfig.tailSuppression.apply = config.TailSuppression()->Apply(); | |
| fBaseConfig.tailSuppression.epsilon = config.TailSuppression()->Epsilon(); | |
| fBaseConfig.tailSuppression.tau = config.TailSuppression()->Tau(); | |
| } | |
| // | |
| // distance-dependent photon survival | |
| // | |
| if (config.DistanceSurvival()) { | |
| fBaseConfig.distanceSurvival.apply = config.DistanceSurvival()->Apply(); | |
| fBaseConfig.distanceSurvival.binEdges = config.DistanceSurvival()->BinEdges(); | |
| fBaseConfig.distanceSurvival.factors = config.DistanceSurvival()->Factors(); | |
| } |
Note : This is being opened against
feature/pmtsim_gainto showcase meaningful differences for review, but it should be rebased on the production branch once #892 is merged.This PR introduces two empirical/effective corrections that close the light model discrepancy for Run-2 by fixing both the signal shape and the overall PE normalization (see SBN-docdb-48157, SBN-docdb-48511).
Two corrections are introduced:
ApplyTailSuppression()applies a subtractive, causal correction to the final waveform. It tracks a running low-pass-filtered estimateQof the waveform (filter with time constantTau) — and subtractsEps x Qfrom each sample (clamped so the pulse cannot flip polarity).Espis a factor that controls the strenght of the correction. This removes the excess integrated charge in the tail while leaving the prompt peak essentially untouched.Distance-dependent photon survival probability: instead of a flat QE cut, each scintillation photon is now kept with probability
QE x S(d), whereS(d)is a function of the distance between the emission point of the photon and the PMT. It is configured via specifying distance bins viaDistanceSurvival.BinEdgesand the corresponding values withDistanceSurvival.Factors. It defaults to 1 outside the configured bin range.Both corrections are currenlty enabled and tuned ONLY for Run-2 data.
The tune for Run-3/4 is still being optimized.