MT Safe L1Topo Simulation Monitoring

Under the code migration for Athena master, we implemented MT safe monitoring for L1Topo simulation as discussed in the following L1Topo commissioning meeting. https://indico.cern.ch/event/912424/contributions/3837365/attachments/2025448/3388182/l1topo_sw_230420.pdf

Basically Gaudi/LockedHandle tool is used in Trigger/TrigT1/L1Topo/L1TopoSimulation/src/AthenaL1TopoHistSvc.cxx and histograms are initialized through the L1TopoAlgorithms by registerHist function as it is done before. Only new fillHist1D and fillHist2D functions are included and proceed for all relevant algorithms to fill the histogram in the event loop.

Although this is done by LockedHandle through the Trigger/TrigT1/L1Topo/L1TopoSimulation/src/AthenaL1TopoHistSvc.cxx in L1TopoSimulation, we kept the StandAlone tool Gaudi independent and filling histograms are proceeding by Trigger/TrigT1/L1Topo/L1TopoCoreSim/Root/StandaloneL1TopoHistSvc.cxx where TH1 and TH2 objects are stored and saved locally.

Job option is in the attach.

Cheers, Anil

cc @orlando , @cmorenom , @stelzer , @iriu

L1TopoSimStandAlone.py

P.S. New job option available in L1TopoSimulation/share/L1TopoSimulationTest.py
Changes are tested by : athena.py --threads=4 L1TopoSimulation/L1TopoSimulationTest.py --evtMax=1000
Related files can be found in this path : /afs/cern.ch/work/a/asonay/public/trig-test/

Hi @asonay thanks for preparing the MR, it looks pretty good to me ! I just have a couple of comments:

• I think the JO you attach in the description is very useful for testing the standalone sim, could you please add it as part of the MR with proper re-naming if necessary and adding the standard copyright statement on top?
• I see that in L1TopoSimulation.cxx you commented out the lines related to the loading of the providers, I guess it’s because these can’t be tested yet? Are you able to run the code in a more automated way by switching this part off if running with EnableInputASCII to true? I don’t think it’s a big problem like it is now but I’d rather have it cleaner if possible, also if we want to keep this EnableInputASCII functionality for the future would be nice to have a proper switch between txt input and AOD-like L1calo/muon inputs (providers).
• Same comment for !32632 (diffs)

Let's wait also for @cmorenom in case he has nay further comments.

Cheers

Nicola

Also, @asonay , could you please remove WIP: and add it back ? In this was we can trigger the CI build to see if any of your change is introducing problems in the tests. Thank you, Nicola

Hi @asonay @orlando I looks good to me! Just one question. I see that now ConfigurableAlg has access to Gaudi functionalities (!32632 (diffs)) but I thought we wanted to keep this part of the code Athena/Gaudi independent. If it's not used in there, could that line be removed please?

added 1 commit

• 7177bf45 - L1TopoSimulation-SA-Feature

Compare with previous version

Hi @orlando, @cmorenom, thank you for pointing out, few changes are implemented by your comments.

• Gaudi functionalities has dropped in ConfigurableAlg since it is unnecessary.
• JO is added.
• The commented out lines in L1TopoSimulation.cxx brought back. Only initial parameter for providers commented out since they are not ready. (They can be initialized in JO)

Additionally I tried to run JO by using offline athena with MT by using the following command, athena L1TopoSimStandAlone.py --evtMax=600 --threads=4 It crash if number of threads different than one. Need to check in detail. I'm temporarily removing WIP

Cheers, Anil

unmarked as a Work In Progress

changed the description

This merge request affects 4 packages:

• Trigger/TrigT1/L1Topo/L1TopoAlgorithms
• Trigger/TrigT1/L1Topo/L1TopoCoreSim
• Trigger/TrigT1/L1Topo/L1TopoInterfaces
• Trigger/TrigT1/L1Topo/L1TopoSimulation

Adding @jchapman ,@vpascuzz ,@kwhalen ,@afaulkne as watchers

added Trigger master review-pending-level-1 labels

CI Result FAILURE (hash 7177bf45)

	Athena	AthSimulation	AnalysisBase	AthGeneration
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
AnalysisBase: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 13953]

marked as a Work In Progress

Removing review labels as this MR is still a WIP... Magdalena (L1)

removed review-pending-level-1 label

added 1 commit

• b394893f - Generic-L1TopoSim

Compare with previous version

Jenkins please retry a build

added 1 commit

• 3ebf64e6 - Independent test for L1TopoSimulation

Compare with previous version

added 1 commit

• e0c1c0ab - Moving HistSvc into source

Compare with previous version

added 1 commit

• 48c41a3b - Removing unused files

Compare with previous version

changed the description

• Txt mode in L1TopoSimulation.cxx removed and turned back to original file.
• New component created in L1TopoSimulation/src/test/ to be able to test Algorithms by using the XML menu and txt input.
• Now --threads works for current JO. (Please see description)

I believe this MR is stable now. Please have a look one last time @cmorenom @orlando @stelzer

added 1 commit

• 7596fd9e - Removing Gaudi lib in algos

Compare with previous version

jenkins please retry

Hi @asonay the MR looks good, let's see if the build goes well and if @cmorenom is fine with it too. Thank you

Hi @asonay the MR looks good to me too. I just have a quick comment, I can see that in StandaloneL1TopoHistSvc.h you added the Gaudi libraries as well (!32632 (diffs) ) but I believe they are not used there no? If not, could they be removed please? Same for IL1TopoHistSvc.h (!32632 (diffs))

After fixing that, you can go ahead and remove the WIP I think! :)

added 1 commit

• 6eb550ec - Removing Gaudi dependency

Compare with previous version

unmarked as a Work In Progress

Thank you for checking!

added 1 commit

• 9c8c2c0d - Resolved merge conflict by rm src/AthenaL1TopoHistSvc

Compare with previous version

marked as a Work In Progress

Turned to the WIP due to merge conflict.

added 11903 commits

• 9c8c2c0d...586d3211 - 11902 commits from branch atlas:master
• 2aeb0d63 - Resolve conflicts

Compare with previous version

Jenkins please retry a build

This merge request affects 4 packages:

• Trigger/TrigT1/L1Topo/L1TopoAlgorithms
• Trigger/TrigT1/L1Topo/L1TopoCoreSim
• Trigger/TrigT1/L1Topo/L1TopoInterfaces
• Trigger/TrigT1/L1Topo/L1TopoSimulation

Adding @jchapman ,@vpascuzz ,@kwhalen ,@afaulkne as watchers

unmarked as a Work In Progress

CI Result SUCCESS (hash 2aeb0d63)

	Athena	AthSimulation	AnalysisBase	AthGeneration
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
AnalysisBase: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 14310]

added review-pending-level-1 label

This MR is ready to be reviewed now.

changed the description

Hi @asonay

I have some minor comments mainly related to the style. Could you have a look ? Otherwise, the changes look good to me.

Thanks.

Xiaozhong (L1)

added review-user-action-required label and removed review-pending-level-1 label

added 1 commit

• f5c5807b - Code cleanup

Compare with previous version