Migrate Monitoring Inclusions
I've made the following changes across the entire repository.
-
#include "AthenaMonitoring/GenericMonitoringTool.h"
-->#include "AthenaMonitoringKernel/GenericMonitoringTool.h"
-
#include "AthenaMonitoring/Monitored.h"
-->#include "AthenaMonitoringKernel/Monitored.h"
-
from AthenaMonitoring.GenericMonitoringTool
-->from AthenaMonitoringKernel.GenericMonitoringTool
Watcher: @fwinkl
Here are the packages which I think only rely on AthenaMonitoringKernel, and could have updated CMake files. But I'll let Frank handle that in a separate MR if he thinks it's worth changing.
Package |
---|
Calorimeter/CaloRec/ |
Control/AthenaExamples/AthExMonitored/ |
HLT/Event/TrigByteStreamCnvSvc/ |
HLT/Trigger/TrigControl/TrigServices/ |
InnerDetector/InDetRecAlgs/InDetPriVxFinder/ |
InnerDetector/InDetRecAlgs/SiSpacePointFormation/ |
MuonSpectrometer/MuonReconstruction/MuonTrackMakers/MuonTrackMakerAlgs/MuonSegmentTrackMaker/ |
Reconstruction/MuonIdentification/MuonCombinedAlgs/ |
Trigger/TrigAlgorithms/TrigCaloRec/ |
Trigger/TrigAlgorithms/TrigEFMissingET/ |
Trigger/TrigAlgorithms/TrigL2MuonSA/ |
Trigger/TrigAlgorithms/TrigMinBias/ |
Trigger/TrigAlgorithms/TrigmuComb/ |
Trigger/TrigAlgorithms/TrigmuIso/ |
Trigger/TrigAlgorithms/TrigMuonEF/ |
Trigger/TrigAlgorithms/TrigmuRoI/ |
Trigger/TrigAlgorithms/TrigT2BeamSpot/ |
Trigger/TrigAlgorithms/TrigT2CaloCommon/ |
Trigger/TrigAlgorithms/TrigT2CaloEgamma/ |
Trigger/TrigAlgorithms/TrigT2MinBias/ |
Trigger/TrigAlgorithms/TrigTauRec/ |
Trigger/TrigHypothesis/TrigBjetHypo/ |
Trigger/TrigHypothesis/TrigBphysHypo/ |
Trigger/TrigHypothesis/TrigEgammaHypo/ |
Trigger/TrigHypothesis/TrigHLTJetHypo/ |
Trigger/TrigHypothesis/TrigMissingETHypo/ |
Trigger/TrigHypothesis/TrigMultiVarHypo/ |
Trigger/TrigHypothesis/TrigMuonHypoMT/ |
Trigger/TrigHypothesis/TrigTauHypo/ |
Trigger/TrigMonitoring/TrigBjetMonitoring/ |
Trigger/TrigMonitoring/TrigBphysMonitoring/ |
Trigger/TrigMonitoring/TrigCaloMonitoring/ |
Trigger/TrigMonitoring/TrigEgammaMonitoring/ |
Trigger/TrigMonitoring/TrigHLTMonitoring/ |
Trigger/TrigMonitoring/TrigMETMonitoring/ |
Trigger/TrigSteer/DecisionHandling/ |
Trigger/TrigSteer/L1Decoder/ |
Trigger/TrigSteer/TrigOutputHandling/ |
Merge request reports
Activity
This merge request affects 51 packages. Since this is a long list, I will not print it here.
Adding @martindl ,@rosati ,@jburr ,@kzoch ,@mronzani ,@xiaozhon ,@battagl ,@wiedenma ,@solodkov ,@bernius ,@khamano ,@stsuno ,@fwinkl ,@oda ,@sekula ,@nkoehler ,@goetz ,@cvarni ,@wleight ,@ibragimo ,@damazio ,@pavol ,@sroe ,@gavrilen ,@harkusha ,@joheinri ,@ssnyder ,@tamartin ,@adbailey ,@ggallard ,@rbielski ,@rbianchi ,@ebergeas as watchers
Hi @cburton,
I'm confused by this change. Could you explain for me the purpose of the two packages AthenaMonitoring and AthenaMonitoringKernel and which one should be linked/imported by users (or if both, in which cases)? I just made an opposite change recently in !28726 (merged) which fixed some cyclic CMake dependencies and I was convinced I was doing the right change. Discussing with @fwinkl back then I had the impression he agreed. However, now it seems this was wrong?Or maybe this is already documented somewhere you could point me to?
Cheers,
RafalAthenaMonitoring: "higher level" base code for the DQ offline monitoring system
AthenaMonitoringKernel: lower level code for histogram creation and managementThese two packages were originally one, but were split in !26089 (merged) in order to break a cyclic dependency AthenaMonitoring-->TrigDecisionTool-->DecisionHandling-->AthenaMonitoring. Then, this allowed me to make the change in this commit, since DecisionHandling should depend only on the low-level code in AthenaMonitoringKernel.
For a full and better answer, I think @fwinkl should comment.
Edited by Charles Burton- Resolved by Rafal Bielski
Oh, I understant now. This comment by Rafal is useful. It appears that DecisionHandling needs AthenaMonitoring after all. However, TrigDecisionTool does not need DecisionHandling. Therefore, the cycle has separately been broken between those two.
In the current master, all of the include statements reference three files
AthenaMonitoring/python/GenericMonitoringTool.py AthenaMonitoring/AthenaMonitoring/GenericMonitoringTool.h AthenaMonitoring/AthenaMonitoring/Monitored.h
However, all these files in AthenaMonitoring did was forward the
#include
orimport
to the appropriate file in AthenaMonitoringKernel, the package in which those three files now live.This MR is probably more cosmetic than anything else. I've left any significant changes to cmake files to Frank, as those sorts of changes are definitely outside my realm of knowledge.
✅ CI Result SUCCESS (hash b06def15)Athena AthSimulation externals ✅ ✅ cmake ✅ ✅ make ✅ ✅ required tests ✅ ✅ optional tests ✅ ✅ Full details available on this CI monitor view
✅ Athena: number of compilation errors 0, warnings 0
✅ AthSimulation: number of compilation errors 0, warnings 0
📝 For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 7798]- Resolved by Frank Winklmeier
This is not a self-consistent MR. If you change the include files you also need to modify the CMakeLists.txt files. So any references to AthenaMonitoring need to be replaced by AthenaMonitoringKernel.
Ok. Based on @fwinkl's last comment, it seems like this would require editing all of the cmake files for trigger. I'll close this and let him handle it in a separate MR.
This merge request affects 51 packages. Since this is a long list, I will not print it here.
Adding @martindl ,@rosati ,@jburr ,@kzoch ,@mronzani ,@xiaozhon ,@battagl ,@wiedenma ,@solodkov ,@bernius ,@khamano ,@stsuno ,@fwinkl ,@oda ,@sekula ,@nkoehler ,@goetz ,@cvarni ,@wleight ,@ibragimo ,@damazio ,@pavol ,@sroe ,@gavrilen ,@harkusha ,@joheinri ,@ssnyder ,@tamartin ,@adbailey ,@ggallard ,@rbielski ,@rbianchi ,@ebergeas as watchers
✅ CI Result SUCCESS (hash 097e3fc7)Athena AthSimulation externals ✅ ✅ cmake ✅ ✅ make ✅ ✅ required tests ✅ ✅ optional tests ✅ ✅ Full details available on this CI monitor view
✅ Athena: number of compilation errors 0, warnings 0
✅ AthSimulation: number of compilation errors 0, warnings 0
📝 For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 7842]- Resolved by Frank Winklmeier
added review-user-action-required label and removed review-pending-level-1 label
This merge request affects 51 packages. Since this is a long list, I will not print it here.
Adding @martindl ,@rosati ,@jburr ,@kzoch ,@mronzani ,@xiaozhon ,@battagl ,@wiedenma ,@solodkov ,@bernius ,@khamano ,@stsuno ,@fwinkl ,@oda ,@sekula ,@nkoehler ,@goetz ,@cvarni ,@wleight ,@ibragimo ,@damazio ,@pavol ,@sroe ,@gavrilen ,@harkusha ,@joheinri ,@ssnyder ,@tamartin ,@adbailey ,@ggallard ,@rbielski ,@rbianchi ,@ebergeas as watchers
added review-pending-level-1 label and removed review-user-action-required label
❎ CI Result FAILURE (hash 951a7c3d)Athena AthSimulation externals ✅ ✅ cmake ✅ ✅ make ✅ ✅ required tests ⭕ ✅ optional tests ✅ ✅ Full details available on this CI monitor view
✅ Athena: number of compilation errors 0, warnings 0
✅ AthSimulation: number of compilation errors 0, warnings 0
📝 For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 7891]This merge request affects 51 packages. Since this is a long list, I will not print it here.
Adding @martindl ,@rosati ,@jburr ,@kzoch ,@mronzani ,@xiaozhon ,@battagl ,@wiedenma ,@solodkov ,@bernius ,@khamano ,@stsuno ,@fwinkl ,@oda ,@sekula ,@nkoehler ,@goetz ,@cvarni ,@wleight ,@ibragimo ,@damazio ,@pavol ,@sroe ,@gavrilen ,@harkusha ,@joheinri ,@ssnyder ,@tamartin ,@adbailey ,@ggallard ,@rbielski ,@rbianchi ,@ebergeas as watchers
❎ CI Result FAILURE (hash 951a7c3d)Athena AthSimulation externals ✅ ✅ cmake ✅ ✅ make ✅ ✅ required tests ⭕ ✅ optional tests ✅ ✅ Full details available on this CI monitor view
✅ Athena: number of compilation errors 0, warnings 0
✅ AthSimulation: number of compilation errors 0, warnings 0
📝 For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 7905]There is a crash in the trigger test (ATR-20743) but that is unrelated to the changes here.
Edited by Frank Winklmeieradded review-approved label and removed review-pending-level-1 label
mentioned in commit 800158de
added sweep:ignore label