Make number of bunch crossings for RPC L1 simulation configurable

As discussed in ATR-28396, the number of bunch crossings being readout by the RPCs changed partway through 2023, but the L1 trigger simulation uses hardcoded values for the number of bunch crossings, and which bunch crossing is the central one. This makes both values configurable via flags so that the L1 simulation can be run on data from both before and after the readout changes.

Overall this is done by setting configurable properties in a few places (the DigitToRDO conversion, the RDO decoder, and the RPC trigger simlation alg), and then using two added flags to configure these properties consistently. The values for the number of bunch crossings and central value are then passed through to other places that need them (the Matrix, sector logic, etc).

One thing to note is that there are a lot of arrays that depend on the number of bunch crossings when setting the array lengths. I assumed that we wouldn't go back to reading out more than 8 bunch crossings, and kept the dependence on the number of bunch crossings on the array lengths in the comments in case it needs to be adjusted in the future. In the end, when we use the arrays, we always loop over up to the configured number of bunch crossings. So we are defining arrays that have a length longer than needed, but the only other way around this would be to move everything to vectors, and that was a larger intervention than I was willing to do for this.

Validation plots added to ATR-28396.

cc @mcorradi

This merge request affects 7 packages:

• MuonSpectrometer/MuonCnv/MuonByteStreamCnvTest
• MuonSpectrometer/MuonCnv/MuonRPC_CnvTools
• MuonSpectrometer/MuonConfig
• Trigger/TrigT1/TrigT1RPChardware
• Trigger/TrigT1/TrigT1RPClogic
• Trigger/TrigT1/TrigT1RPCsteering
• Trigger/TriggerCommon/TriggerJobOpts

Affected files list will not be printed in this case

Adding @kluit ,@martindl ,@fwinkl ,@jchapman ,@jojungge ,@rosati ,@fpastore ,@pscholer ,@apsallid ,@stavrop as watchers

added 24.0 MuonSpectrometer Trigger full-unit-tests review-pending-level-1 labels

added Run2-MCReco-output-changed Run3-MCReco-output-changed labels

CI Result FAILURE (hash 32f3db76)

	Athena	AthSimulation
externals
cmake
make
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
For experts only: Jenkins output [CI-MERGE-REQUEST-EL9 5453] (remote access info)

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

mentioned in merge request !69331 (merged)

removed Run2-MCReco-output-changed label

removed Run3-MCReco-output-changed label

added 1 commit

• 43ee43b2 - Pass number of bunch crossings to CMA readout and fix some configuration mistakes

Compare with previous version

This merge request affects 7 packages:

• MuonSpectrometer/MuonCnv/MuonByteStreamCnvTest
• MuonSpectrometer/MuonCnv/MuonRPC_CnvTools
• MuonSpectrometer/MuonConfig
• Trigger/TrigT1/TrigT1RPChardware
• Trigger/TrigT1/TrigT1RPClogic
• Trigger/TrigT1/TrigT1RPCsteering
• Trigger/TriggerCommon/TriggerJobOpts

Affected files list will not be printed in this case

Adding @stavrop ,@fwinkl ,@rosati ,@apsallid ,@pscholer ,@jchapman ,@fpastore ,@jojungge ,@martindl ,@kluit as watchers

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

added Run2-DataOverlay-output-changed Run2-MCOverlay-output-changed Run3-MCOverlay-output-changed labels

CI Result FAILURE (hash 43ee43b2)

	Athena	AthSimulation
externals
cmake
make
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
For experts only: Jenkins output [CI-MERGE-REQUEST-EL9 5638] (remote access info)

CI failure due to FPE. Probably need to add some gaurds for the divisions that were added. L1

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

Also some output has changed, is this expected?

removed Run2-DataOverlay-output-changed label

removed Run2-MCOverlay-output-changed label

removed Run3-MCOverlay-output-changed label

resolved all threads

added 235 commits

• 43ee43b2...2c6fc1c5 - 231 commits from branch atlas:24.0
• 2553d561 - Make number of bunch crossings for RPC simulation configurable
• 4f3b8dff - Pass number of bunch crossings to CMA readout and fix some configuration mistakes
• b92b5981 - Pass NOBXS through to some missing places
• ff126523 - Change properties from unsigned int to int to avoid some odd conversions

Compare with previous version

This merge request affects 7 packages:

• MuonSpectrometer/MuonCnv/MuonByteStreamCnvTest
• MuonSpectrometer/MuonCnv/MuonRPC_CnvTools
• MuonSpectrometer/MuonConfig
• Trigger/TrigT1/TrigT1RPChardware
• Trigger/TrigT1/TrigT1RPClogic
• Trigger/TrigT1/TrigT1RPCsteering
• Trigger/TriggerCommon/TriggerJobOpts

Affected files list will not be printed in this case

Adding @pscholer ,@jojungge ,@jchapman ,@stavrop ,@fwinkl ,@apsallid ,@rosati ,@kluit ,@martindl ,@fpastore as watchers

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

CI Result SUCCESS (hash ff126523)

	Athena	AthSimulation
externals
cmake
make
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
For experts only: Jenkins output [CI-MERGE-REQUEST-EL9 6031] (remote access info)

Just one small point to clarify, otherwise looks good.

Best, Thomas - L1

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

added 1 commit

• 7d7a49f4 - Add error messages to prevent configuring NOBXS>8

Compare with previous version

resolved all threads

This merge request affects 7 packages:

• MuonSpectrometer/MuonCnv/MuonByteStreamCnvTest
• MuonSpectrometer/MuonCnv/MuonRPC_CnvTools
• MuonSpectrometer/MuonConfig
• Trigger/TrigT1/TrigT1RPChardware
• Trigger/TrigT1/TrigT1RPClogic
• Trigger/TrigT1/TrigT1RPCsteering
• Trigger/TriggerCommon/TriggerJobOpts

Affected files list will not be printed in this case

Adding @rosati ,@martindl ,@pscholer ,@kluit ,@jojungge ,@fpastore ,@stavrop ,@jchapman ,@fwinkl ,@apsallid as watchers

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

CI Result FAILURE (hash 7d7a49f4)

	Athena	AthSimulation
externals
cmake
make
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
For experts only: Jenkins output [CI-MERGE-REQUEST-EL9 6092] (remote access info)

679/679 Test #179: AthExHive_AthExHiveCond_test_ctest ........................................***Timeout 300.05 sec

shows up in AthSimulation unit tests

This doesn't seem to be related to your changes, @sshaw?

L1 shifter

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

No I don't think the timeout can be related to the changes. I tried locally running the test with/without the changes in the MR and it took the same amount of time for both (about a minute). In the previous CI round, the time for the test also seemed pretty reasonable, and I didn't change anything that would impact this in between.

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

Changes look good from L1 side, unit test timeout is unrelated based on local tests by developer - approving

L1 shifter

added review-approved label and removed review-pending-level-1 label

added review-approved-point1 label

mentioned in commit 15068d62

merged

mentioned in merge request !69668 (merged)