v492-dev17#351
Merged
Merged
Conversation
calculateTraceSum_slidingWindow() fRaw branch accumulated the raw charge sum in 'charge' but returned FADC[1] (second sample, ped-subtracted) instead. Replace with 'return charge'. Fixes A2 in issue #352. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
calculateTraceSum_slidingWindow() fRaw branch accumulated the raw charge sum in 'charge' but returned FADC[1] (second sample, ped-subtracted) instead. Replace with 'return charge'. Fixes A2 in issue #352. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A2: fix sliding-window fRaw branch returning FADC[1] instead of charge
…t type Non-XGB effective-area and stereo-analysis jobs were unconditionally opening both XGB friend files (stereo + gamma-hadron) whenever their suffix strings were non-empty, even when the active reconstruction method and gamma-hadron cut type did not require them. Changes: - CData: store data file name in fDataFileName for deferred tree loading - CData::loadGHXGBTree(): new idempotent method to load the GH XGB friend tree on demand, after the analysis type is known - VStereoAnalysis::getDataFromFile(): pass stereo suffix only when fEnergyReconstructionMethod==2 or fDirectionReconstructionMethod==2; always pass empty GH suffix (deferred loading) - VStereoAnalysis::fillHistograms(): call loadGHXGBTree() after setCuts() if and only if fCuts->useXGBoostCuts() is true - makeEffectiveArea.cpp: guard both suffixes at construction time; fCuts->readCuts() is already called before CData is built Fixes: #352 (item E4) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fix(E4): guard XGB sidecar file opens by reconstruction method and cut type
…lar training BoostType=Grad does not support InverseBoostNegWeights (TMVA's global default for BDT), which caused TMVA to silently replace it with Pray, making the effective training configuration differ from the nominal one. Explicitly setting NegWeightTreatment=Pray in the default options string eliminates the silent substitution and ensures the logged configuration matches actual training behaviour. Fixes: #352 (item from mvaAngRes log) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fix: explicit NegWeightTreatment=Pray for BoostType=Grad in TMVA angular training
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses multiple correctness and robustness bugs reported in #352 across trace handling, array reconstruction, IRF/effective-area tooling, and TMVA/XGB-based workflows. It also tightens some diagnostics and updates documentation/changelog references.
Changes:
- Fixes several physics-impacting logic/edge-case bugs (trace timing OOB, sliding-window charge return, core-error formula, NaN screening, pair-angle selection, emission-height weighting/geometry).
- Improves ML/TMVA and XGB friend-tree handling (proper TMVA regression factory config, explicit negative-weight treatment, deferred GH friend-tree loading, avoid probing XGB sidecars when not needed).
- Updates user-facing messages and docs fragments/changelog links.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/VTraceHandler.cpp | Fix pulse-timing OOB edge case; correct sliding-window raw-branch return; guard sliding-window indexing. |
| src/VTMVARunData.cpp | Clarify configuration error messages for zenith/energy bin sanity checks. |
| src/VTableLookupRunParameter.cpp | Fix CLI help text to match actual accepted -fill values (1/0). |
| src/VStereoAnalysis.cpp | Defer GH XGB friend-tree loading until cut type is known; avoid opening GH sidecars too early. |
| src/VInstrumentResponseFunctionRunParameter.cpp | Add explicit handling for unknown run-parameter keys during text parsing. |
| src/VInstrumentResponseFunctionData.cpp | Fix core-error formula to use both Xcore and Ycore. |
| src/VImageParameterCalculation.cpp | Guard muon-ring size correction against division by zero; set valid Minuit bounds in LL fit. |
| src/VEmissionHeightCalculator.cpp | Guard emission-height weights against invalid log10(size); clear telescope-position vectors on reinit. |
| src/VCalibrationData.cpp | Fix low-gain average-T0 setter to not overwrite high-gain value. |
| src/VArrayAnalyzer.cpp | Fix core validity check to test yimp; fix method-4 pair-angle cut to use the current telescope pair. |
| src/trainTMVAforGammaHadronSeparation.cpp | Fix zenith-bin bounds check; configure TMVA Factory as Classification vs Regression appropriately. |
| src/trainTMVAforAngularReconstruction.cpp | Remove unexplained 0.8 scaling of train/test counts; make negative-weight treatment explicit in options. |
| src/printRunParameter.cpp | Return failure exit code when input file is missing. |
| src/makeEffectiveArea.cpp | Avoid opening XGB sidecar files unless needed; improve ROOT file handling in MC-histogram aggregation. |
| src/CData.cpp | Store data filename for deferred GH friend-tree load; robust .root suffix handling; add loadGHXGBTree(). |
| inc/CData.h | Add stored data filename and declaration for deferred GH friend-tree loading. |
| src/anasum.cpp | Fix dead conditional in stereo-analysis mode selection. |
| README.md | Minor wording update and add CONTRIBUTORS link. |
| CHANGELOG.md | Fix repository releases link. |
| docs/changes/351.bugfix.md | Add towncrier fragment for A2 fix (trace-sum sliding-window raw branch). |
| docs/changes/352.bugfix.md | Add towncrier fragment documenting conditional XGB sidecar probing. |
| docs/changes/353.bugfix.md | Add towncrier fragment documenting explicit TMVA negative-weight option for Grad BDT. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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.
Addresses bugs reported in #352