Fix MM digi identifier warnings (ATLASSIM-5822)

Fixed an issue in Micromega digitization where digits were sometimes assigned an incorrect identifier. The identifier in question was always a MDT identifier and caused some digits to be discarded.

The underlying issue was caused by an identifier being initialized and not always assigned to a value before being used. We have made sure that the identifier is now defined with a value.

The issue also came from HITS with exactly 0 kinetic energy. The MM experts decided it was best to also simply ignore these HITS as they may cause issues in the logic of the ionization simulations. These HITS so far only seem to appear in pileup jobs and the rate of such particles is very small to be negligible to the end result (about a hundred pileup HITS per 1000 events).

See ATLASSIM-5822 for more details and !52947 (closed) where it was also discussed. Tests are currently underway to see if it solves FPE warnings in some MM L1 trigger jobs.

tagging @chchau @pscholer @rosati

added Digitization MuonSpectrometer master labels

This merge request affects 1 package:

• MuonSpectrometer/MuonDigitization/MM_Digitization

This merge request affects 1 file:

• MuonSpectrometer/MuonDigitization/MM_Digitization/src/MM_DigitizationTool.cxx

Adding @jchapman ,@rosati ,@wleight ,@stavrop ,@egodden ,@jojungge as watchers

added review-pending-level-1 label

Hi @alaurier,

After MM_DigitizationTool::combinedStripResponseAllHits(...) is called is it worth adding some kind of sanity check on the output of MM_ElectronicsToolInput::digitID()?

Cheers,

John

added 1 commit

• 012323f1 - Fix MM digi identifier warnings

Compare with previous version

This merge request affects 1 package:

• MuonSpectrometer/MuonDigitization/MM_Digitization

This merge request affects 1 file:

• MuonSpectrometer/MuonDigitization/MM_Digitization/src/MM_DigitizationTool.cxx

Adding @jchapman ,@rosati ,@wleight ,@stavrop ,@egodden ,@jojungge as watchers

To give some sort of report on the result on this MR on pileup presampling jobs where these warnings showed up.

The changes proposed in this MR reduced the number of identifier warnings in the presampling of 1000 events from 218 down to zero.

I will have confirmation by May 11th if this also fixes the FPE warnings mentioned in !52947 (closed) .

CI Result SUCCESS (hash f31452d3)

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

CI Result SUCCESS (hash 012323f1)

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

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

Changes and CI look fine from L1 point of view. Will move to user action required until last checks are performed.

Kira (L1)

resolved all threads

Hi @alaurier is there any other check to be made before this can be merged ? From the thread it's not obvious to me that there are pending issues still

cheers, Tommaso

Hi @tlari , I'm happy with the current checks that the warnings disappear for pileup jobs. The check for FPE warnings I think can be treated as an other issue and discussed in another MR / JIRA. I'll remove the user-action label.

removed review-user-action-required label

mentioned in merge request !53149 (merged)

Changes look good from L1 side - approving.

@alaurier, for the future: If you remove review-user-action-required but don't push any new commits, please remember to re-attach review-pending-level-1, because otherwise shifters might miss that the MR is ready. If you would be pushing new commits, the label would get re-attached automatically when the CI restarts, but here it is not the case.

Jakub (L1 shifter)

added review-approved label

added urgent label

Adding urgent label, since my understand is that this prevent what might be a significant rate of FPE crashes which would affect the MC overlay physics validation jobs which we plan to run with the release which will hopefully be built from the next nightly

Tommaso