MuonSelectionTool- remove caching variables for LowPtMVA WP
Hi,
although this MR looks again quite heavy. The changes are relatively small and easy. The first commit applies a standardized clang-format to the package while the remaining ones apply the actual changes to the tool, which is removing the mutable data pointers to parse the LowPt MVA selection.
Merge request reports
Activity
This merge request affects 1 package:
- PhysicsAnalysis/MuonID/MuonSelectorTools
This merge request affects 13 files:
- PhysicsAnalysis/MuonID/MuonSelectorTools/ATLAS_CHECK_THREAD_SAFETY
- PhysicsAnalysis/MuonID/MuonSelectorTools/MuonSelectorTools/MuonSelectionTool.h
- PhysicsAnalysis/MuonID/MuonSelectorTools/MuonSelectorTools/MuonSelectorToolsDict.h
- PhysicsAnalysis/MuonID/MuonSelectorTools/MuonSelectorTools/errorcheck.h
- PhysicsAnalysis/MuonID/MuonSelectorTools/Root/MuonSelectionTool.cxx
- PhysicsAnalysis/MuonID/MuonSelectorTools/scripts/plotTightWPMaps.C
- PhysicsAnalysis/MuonID/MuonSelectorTools/src/MuonQualityUpdaterAlg.cxx
- PhysicsAnalysis/MuonID/MuonSelectorTools/src/MuonQualityUpdaterAlg.h
- PhysicsAnalysis/MuonID/MuonSelectorTools/src/MuonSelectionAlg.cxx
- PhysicsAnalysis/MuonID/MuonSelectorTools/src/MuonSelectionAlg.h
- PhysicsAnalysis/MuonID/MuonSelectorTools/src/components/MuonSelectorTools_entries.cxx
- PhysicsAnalysis/MuonID/MuonSelectorTools/test/ut_MuonSelectorToolsTester_data.cxx
- PhysicsAnalysis/MuonID/MuonSelectorTools/util/MuonSelectorToolsTester.cxx
Adding @jojungge as watcher
added Analysis analysis-review-required master review-pending-level-1 labels
Hi, @jojungge
Are you using MuonSelectorToolsTester to test your changes? I tried now, and immediately got a crash:
: Booking "BDTG" of type "BDT" from /cvmfs/atlas.cern.ch/repo/sw/database/GroupData/MuonSelectorTools/190118_PrelimLowPtMVA/LowPtMVA_Weights/BDTG_9JAN2019_MuidCB_EVEN.weights.xml. : Reading weight file: /cvmfs/atlas.cern.ch/repo/sw/database/GroupData/MuonSelectorTools/190118_PrelimLowPtMVA/LowPtMVA_Weights/BDTG_9JAN2019_MuidCB_EVEN.weights.xml <FATAL> : Dataset[Default] : You declared 0 variables in the Reader while there are 8 variables declared in the file ***> abort program execution terminate called after throwing an instance of 'std::runtime_error' what(): FATAL error Aborted (core dumped)Edited by Magnar Kopangen Bugge-
Athena AthSimulation AthGeneration AnalysisBase AthAnalysis DetCommon externals 
cmake make required tests 
optional tests Full details available on this CI monitor view
-
This merge request affects 2 packages:
- PhysicsAnalysis/Interfaces/MuonAnalysisInterfaces
- PhysicsAnalysis/MuonID/MuonSelectorTools
Affected files list will not be printed in this case
Adding @fsforza ,@nakahama ,@szambito ,@jojungge ,@markowen ,@mkbugge ,@akraszna ,@mvanadia ,@gabarone ,@gartoni as watchers
added Trigger label
added 1 commit
- ccee9197 - Actually I do not know what the smart TMVA constructor does with the parsed vector
-
This merge request affects 2 packages:
- PhysicsAnalysis/Interfaces/MuonAnalysisInterfaces
- PhysicsAnalysis/MuonID/MuonSelectorTools
Affected files list will not be printed in this case
Adding @fsforza ,@nakahama ,@szambito ,@jojungge ,@markowen ,@mkbugge ,@akraszna ,@mvanadia ,@gabarone ,@gartoni as watchers
removed analysis-review-required label
-
Athena AthSimulation AthGeneration AnalysisBase AthAnalysis DetCommon externals cmake make required tests optional tests Full details available on this CI monitor view
-
Athena AthSimulation AthGeneration AnalysisBase AthAnalysis DetCommon externals cmake make required tests optional tests Full details available on this CI monitor view
Hi, @jojungge
I tried to test your changes again, and while they do not crash now, the number of muons passing the Low-pT MVA working point seems to change dramatically. Obviously your changes were meant to be technical and should not change the physics results. Could you please mark the MR WIP until you/we can investigate further and sort this out?
I recommend using the MuonSelectorToolsTester at every step of development, making sure that physics output does not change if changes are technical. (Note that physics changes are not flagged in the unit tests, as we currently do not have a reference file. This was discussed in the past, but could certainly be revisited. Right now, though, we don't so far have a solid ASG_TEST_FILE to work with in rel. 22. Another thing is that the MuonSelectorToolsTester by default prints results for the cut-based low-pT working point, so you have to change the configuration to test these changes. I am planning some small updates to the tester, and we could of course easily print results for both versions.)
removed review-pending-level-1 label