Bugfix: opt argument of defineHistogram
In the existing code, a user-provided dictionary of options produces a crash. In this MR, the bug is fixed, and appropriate tests are added to check the relevant code blocks in the future.
Merge request reports
Activity
This merge request affects 1 package:
- Control/AthenaMonitoringKernel
This merge request affects 2 files:
- Control/AthenaMonitoringKernel/python/GenericMonitoringTool.py
- Control/AthenaMonitoringKernel/test/test_defineHistogram.py
Adding @ssnyder as watcher
added Core DQ master review-pending-level-1 labels
-
Athena AthSimulation AthGeneration AnalysisBase AthAnalysis DetCommon externals cmake 
make 
required tests optional tests Full details available on this CI monitor view. Check the JIRA CI status board for known problems
Resolved by Charles BurtonWell, there's a compiler warning:
GenericMonitoringTool.py:258:40: warning: E721 do not compare types, use 'isinstance()' typeValid = [type(opt[option]) is type(settings[option]) for option in opt] ^When writing this MR, I originally tried use the function
isinstance. However, I found in testing thatisinstance(True, int)returnsTrue. Therefore, if the user provides a boolean for a histogram option which expects an integer, the code in GMT.py which validates the user's inputs would not catch the error.The MR as written avoids this issue.
I'm unsure what the best/preferred option is.
- Last reply by Charles Burton
added review-user-action-required label and removed review-pending-level-1 label
added 1 commit
- 2b4fa4ff - Create function to validate users' opt inputs
-
This merge request affects 1 package:
- Control/AthenaMonitoringKernel
This merge request affects 2 files:
- Control/AthenaMonitoringKernel/python/GenericMonitoringTool.py
- Control/AthenaMonitoringKernel/test/test_defineHistogram.py
Adding @ssnyder as watcher
added review-pending-level-1 label and removed review-user-action-required label
-
Athena AthSimulation AthGeneration AnalysisBase AthAnalysis DetCommon externals cmake make required tests 
optional tests Full details available on this CI monitor view. Check the JIRA CI status board for known problems
- Resolved by Tomasz Bold
The new issue is with InDetPriVxFinderMonitoring Line 20. An invalid option is being provided. The validation code I am adding in this MR is throwing an error because of that.
I can remove
opt='kLBN'from the histogram definition, and this will not change any functionality. (Tagging @tbold, in case you happen to know what the intended purpose of that histogram is.) - Last reply by Tomasz Bold
added 1 commit
- 8939a5c1 - Remove unsupported option from histogram definition
added InnerDetector label
-
Athena AthSimulation AthGeneration AnalysisBase AthAnalysis DetCommon externals cmake make required tests optional tests Full details available on this CI monitor view. Check the JIRA CI status board for known problems
Unfortunately, the CI still fails:
Traceback (most recent call last): File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc11-opt/bin/Run3DQTestingDriver.py", line 127, in <module> dq = AthenaMonitoringCfg(ConfigFlags) File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc11-opt/python/AthenaMonitoring/AthenaMonitoringCfg.py", line 64, in AthenaMonitoringCfg result.merge(TrigHLTMonTopConfig(flags)) File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc11-opt/python/TrigHLTMonitoring/TrigHLTMonitorAlgorithm.py", line 68, in TrigHLTMonTopConfig result.merge(TrigJetMonConfig(inputFlags)) File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc11-opt/python/TrigJetMonitoring/TrigJetMonitorAlgorithm.py", line 336, in TrigJetMonConfig offlineMonitorConf.toAlg(helper) File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc11-opt/python/JetMonitoring/JetMonitoringConfig.py", line 557, in toAlg tconf.defineHisto(alg, monhelper, path) File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc11-opt/python/JetMonitoring/JetMonitoringConfig.py", line 372, in defineHisto group.defineHistogram(name, path=path, **hargs) File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc11-opt/python/AthenaMonitoringKernel/GenericMonitoringTool.py", line 63, in defineHistogram self._coreDefine(defineHistogram, *args, **kwargs) File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc11-opt/python/AthenaMonitoringKernel/GenericMonitoringTool.py", line 58, in _coreDefine toadd = deffunc(*args, **kwargs) File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc11-opt/python/AthenaMonitoringKernel/GenericMonitoringTool.py", line 450, in defineHistogram settings.update(_options(opt)) File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc11-opt/python/AthenaMonitoringKernel/GenericMonitoringTool.py", line 289, in _options assert len(unknown)==0,\ AssertionError: Unknown option(s) provided: colz.Could you have another look?
Cheers, Martin (L1)
added review-user-action-required label and removed review-pending-level-1 label
added 149 commits
-
8939a5c1...ce946612 - 147 commits from branch
atlas:master - 9b586d9e - Merge branch 'master' of https://gitlab.cern.ch:8443/atlas/athena into master-bugfixGMT
- b19223a6 - Remove invalid defineHistogram option from jet monitoring
-
8939a5c1...ce946612 - 147 commits from branch
added JetEtmiss Reconstruction analysis-review-required review-pending-level-1 labels and removed review-user-action-required label
-
Athena AthSimulation AthGeneration AnalysisBase AthAnalysis DetCommon externals cmake make required tests optional tests Full details available on this CI monitor view. Check the JIRA CI status board for known problems
added review-approved label and removed review-pending-level-1 label
added analysis-review-approved label and removed analysis-review-required label
mentioned in commit 4f5242ba
added sweep:ignore label
