Modify RPC isFront function to match criteria with will be modified RPC geometries for Run, Run 3 and Run 4

[eigen1907]

PR description:

This PR modifies the isFront function inside the RPC Endcap Geometry Builder.
It will be applied in synchronization with PR (#47134) by Sunanda Banerjee.

The isFront function classifies RPCs as front or back chambers by RPC Ids.

As the PR (#47134) modifies the z coordinates of RPCs inside RPC endcap stations (RE-4, RE-3, RE-2, RE+2, RE+3, RE+4),
The criteria for the isFront function needs to be changed as well.
Therefore, this PR modifies the criteria for the isFront function correctly as the z coordinate changes.

For history and more details, see the previous meeting materials.
(https://indico.cern.ch/event/1475698/contributions/6214667/attachments/2962024/5210467/RPCGeomReport_241106RPCDPG_JShin.pdf)

PR validation:

To avoid errors, this PR should be used with #47134 and the new scenario.
Otherwise (when applying this PR alone), the error below will be raised by the selfTest function in the Geometry Builder.

Error reproduction)

A fatal system signal has occurred: abort signal
The following is the call stack containing the origin of the signal.
...
#9  0x00007f62d8e9276c in MuRingForwardDoubleLayer::selfTest() const () from /eos/home-j/joshin/workspace-eos/rpc-geometry/update-isfront/CMSSW_15_0_0_pre1/lib/el9_amd64_gcc12/libRecoMuonDetLayers.so
#10 0x00007f62d8e9359b in MuRingForwardDoubleLayer::MuRingForwardDoubleLayer(std::vector<ForwardDetRing const*, std::allocator<ForwardDetRing const*> > const&, std::vector<ForwardDetRing const*, std::allocator<ForwardDetRing const*> > const&) () from /eos/home-j/joshin/workspace-eos/rpc-geometry/update-isfront/CMSSW_15_0_0_pre1/lib/el9_amd64_gcc12/libRecoMuonDetLayers.so
#11 0x00007f62d8e9f36f in MuonRPCDetLayerGeometryBuilder::buildLayer(int, std::vector<int, std::allocator<int> > const&, int, int, std::vector<int, std::allocator<int> >&, RPCGeometry const&) () from /eos/home-j/joshin/workspace-eos/rpc-geometry/update-isfront/CMSSW_15_0_0_pre1/lib/el9_amd64_gcc12/libRecoMuonDetLayers.so
#12 0x00007f62d8e9fea8 in MuonRPCDetLayerGeometryBuilder::buildEndcapLayers(RPCGeometry const&) () from /eos/home-j/joshin/workspace-eos/rpc-geometry/update-isfront/CMSSW_15_0_0_pre1/lib/el9_amd64_gcc12/libRecoMuonDetLayers.so
#13 0x00007f62bb85a0bf in MuonDetLayerGeometryESProducer::produce(MuonRecoGeometryRecord const&) () from /cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_15_0_0_pre1/lib/el9_amd64_gcc12/pluginRecoMuonDetLayersPlugins.so
...

This change will also require a new payload for RPC Geometry when working with Global Tags.

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

Nothing special

[cmsbuild]

cms-bot internal usage

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47138/43365

[cmsbuild]

A new Pull Request was created by @eigen1907 for master.

It involves the following packages:

• RecoMuon/DetLayers (reconstruction)

@cmsbuild, @jfernan2, @mandrenguyen can you please review it and eventually sign? Thanks.
@CeliaFernandez, @Fedespring, @HuguesBrun, @abbiendi, @andrea21z, @bellan, @cericeci, @jhgoh, @missirol, @rociovilar, @trocino this is something you requested to watch as well.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

[perrotta]

please test
(#47134 is already merged in the IBs)

[bsunanda]

The new scenarios are in #47158 (Run2), #47159 (Run3) and #47160 (Run4).

[perrotta]

please abort

[perrotta]

test parameters:

• pull_requests = Run3-gex186A Modify all Run2 scenarios to include the modified RPC geometry #47158, Run3-gex186B Modify all Run3 scenarios to include the modified RPC geometry #47159, Phase2-gex186C Modify all Run4 scenarios to include the modified RPC geometry #47160

[perrotta]

please test

[cmsbuild]

-1

Failed Tests: UnitTests RelVals RelVals-INPUT AddOn
Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-09a151/43910/summary.html
COMMIT: 185af90
CMSSW: CMSSW_15_0_X_2025-01-21-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/47138/43910/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found 2 errors in the following unit tests:

---> test test_MC_22_crosscheck had ERRORS
---> test test_MC_23_crosscheck had ERRORS

RelVals

• 135.4A fatal system signal has occurred: abort signal
• 1306.0A fatal system signal has occurred: abort signal
• 7.3A fatal system signal has occurred: abort signal

Expand to see more relval errors ...

• 140.045
• 10224.0
• 4.53
• 5.1
• 312.0
• 145.713
• 145.301
• 13234.0
• 1000.0
• 16834.0
• 11634.0
• 136.874
• 9.0
• 145.202
• 14234.0
• 8.0
• 136.731
• 13034.0
• 136.793
• 141.042
• 12846.0
• 4.22
• 25.0
• 25202.0
• 145.408
• 1330.0
• 250202.181
• 145.014
• 14034.0
• 145.104
• 139.001
• 140.56
• 1001.0
• 12834.7
• 12434.0
• 145.5
• 12834.0
• 145.604

RelVals-INPUT

• 145.711145.711_RunParkingSingleMuon02024I/step2_RunParkingSingleMuon02024I.log
• 1030.01030.0_RunHLTPhy2017B/step2_RunHLTPhy2017B.log
• 145.7145.7_RunBTagMu2024I/step2_RunBTagMu2024I.log

Expand to see more relval errors ...

• 145.31
• 1052.0
• 1052.1
• 145.114
• 145.109
• 142.0
• 145.209
• 145.611
• 145.01
• 145.612
• 145.11
• 145.409
• 145.408
• 145.613
• 145.401
• 145.614
• 145.403
• 145.402
• 145.405
• 145.404
• 145.407
• 145.406
• 144.6
• 1002.0
• 1002.3
• 1002.4
• 138.5
• 138.4
• 138.2
• 138.1
• 142.903
• 142.902
• 142.901
• 145.414
• 145.412
• 145.413
• 136.897
• 136.899
• 1000.0
• 145.714
• 145.009
• 145.008
• 145.713
• 145.712
• 145.005
• 145.004
• 145.007
• 145.006
• 145.001
• 145.609
• 145.003
• 145.002
• 145.61
• 145.1
• 1040.0
• 2500.242
• 134.813
• 145.702
• 145.703
• 145.701
• 145.706
• 145.707
• 145.704
• 145.0
• 145.012
• 145.013
• 145.708
• 145.011
• 145.014
• 140.57
• 145.311
• 145.313
• 145.312
• 145.314
• 145.71
• 134.0
• 145.306
• 145.307
• 145.304
• 145.305
• 145.302
• 145.303
• 145.301
• 145.308
• 145.309
• 140.61
• 136.733
• 145.41
• 145.705
• 141.901
• 141.902
• 145.608
• 143.911
• 145.603
• 145.602
• 145.601
• 145.607
• 145.606
• 145.605
• 145.604
• 145.709
• 145.51
• 145.411
• 145.513
• 145.512
• 145.511
• 145.514
• 4.6
• 145.106
• 143.902
• 1001.4
• 143.901
• 145.113
• 145.112
• 145.111
• 145.208
• 145.207
• 145.206
• 145.205
• 145.204
• 145.203
• 145.202
• 145.201
• 1001.3
• 1001.0
• 1005.0
• 145.501
• 145.502
• 145.503
• 145.504
• 145.505
• 145.506
• 145.507
• 145.508
• 145.509
• 4.22
• 145.21
• 136.902
• 136.903
• 136.901
• 1003.0
• 136.904
• 145.108
• 145.2
• 145.104
• 145.105
• 140.56
• 145.107
• 1001.2
• 145.101
• 145.102
• 145.103
• 145.214
• 145.6
• 145.5
• 145.4
• 145.3
• 145.211
• 145.212
• 145.213

AddOn Tests

• fastsim

A fatal system signal has occurred: abort signal

• fastsim1

A fatal system signal has occurred: abort signal

• fastsim2

A fatal system signal has occurred: abort signal

Expand to see more addon errors ...

• hlt_mc_Fake
• hlt_mc_Fake1
• hlt_mc_Fake2
• hlt_mc_GRun
• hlt_mc_GRun
• hlt_mc_HIon
• hlt_mc_HIon
• hlt_mc_PIon
• hlt_mc_PRef
• hlt_mc_PRef
• hlt_mc_Special
• hlt_mc_Special
• hlt_data_Fake
• hlt_data_Fake1
• hlt_data_Fake2
• hlt_data_GRun
• hlt_data_GRun
• hlt_data_HIon
• hlt_data_PIon
• hlt_data_PRef
• hlt_data_PRef
• hlt_data_Special
• hlt_data_Special

[jhgoh]

The errors such as the WF 140.045 are expected

cmsRun: src/RecoMuon/DetLayers/src/MuRingForwardDoubleLayer.cc:196: void MuRingForwardDoubleLayer::selfTest() const: Assertion `frontz < backz' failed.

already written in the PR description.

[jfernan2]

@bsunanda has this been discussed in a PPD or SIM meeting as you requested?

[civanch]

This PR was discussed at SIM meeting (https://indico.cern.ch/event/1507600/) and muon meeting (https://indico.cern.ch/event/1511920/). We agree that this PR is valid, and should be merged to 15_1. It is needed to disable isFront() check for RPC for the time being.

[perrotta]

The suggestion proposed in the muon meeting was about implementing an if-then-else structure, either ruled by a modifier or by a check in the innermost RPC layer, so that the isFront check can be maintained and specialized to the old and updated geometries, respectively. Is there any plan to proceed as such?

[jhgoh]

The suggestion proposed in the muon meeting was about implementing an if-then-else structure, either ruled by a modifier or by a check in the innermost RPC layer, so that the isFront check can be maintained and specialized to the old and updated geometries, respectively. Is there any plan to proceed as such?

Thank you for your suggestions. One of the my concern was adding too much modifications in the base-classes regarding general geometry and related modules, only to add hard-coded exceptional buggy cases.
In any case, if we really want to fix this wrong-staggering only for late-2025 geometry but keep the past geometries as buggy as is, I think it is possible.

I'm writing some notes for @eigen1907 to let him get some idea:

The functions of MuonRPCDetLayerGeometryBuilder class are all static ones (see https://github.com/cms-sw/cmssw/blob/master/RecoMuon/DetLayers/src/MuonRPCDetLayerGeometryBuilder.h#L29-L43), so it is not a good idea to set era-dependancy flag as its member variable.

The one you have to look at/modify is: MuonRPCDetLayerGeometryBuilder::buildEndcapLayers(const RPCGeometry& geo) which is called by MuonDetLayerGeometryESProducer (https://github.com/cms-sw/cmssw/blob/master/RecoMuon/DetLayers/plugins/MuonDetLayerGeometryESProducer.cc#L99).
In the constructor of the MuonDetLayerGeometryESProducer a ParameterSet is passed, so that we can give era-dependent flag with the era-modifiers. Then you can pass additional flag to apply corrected even/odd rule in building the endcap layers. https://github.com/cms-sw/cmssw/blob/master/RecoMuon/DetLayers/plugins/MuonDetLayerGeometryESProducer.cc#L99

[perrotta]

@jhgoh @eigen1907 the suggestion made at the muon meeting (not by me) of looking at the innermost ring and then use it to decide which kind of geometry order has to be considered for the rest of the RPC was also valid, and it does not require defining any era.

[civanch]

Hi all, If the method isFront() is needed for reconstruction of other run time purpose, I would agree with last comment of @perrotta that internal check may be done and modifier is not needed. However, my understanding (may be wrong) is that this method was design for internal checks after geometry construction and is not used run time. If it is true, let us merge this PR and resolve RPC geometry problems - we are limited in time and the best to me is for the time being to disable these checks. It is possible to create an issue for better implementation of isFront() method and do that later.

[cmsbuild]

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47138/43663

Code check has found code style and quality issues which could be resolved by applying following patch(s)

• code-format:
  https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47138/43663/code-format.patch
  e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47138/43663/code-format.patch | patch -p1
  You can also run scram build code-format to apply code format directly

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47138/43668

[cmsbuild]

Pull request #47138 was updated. @atpathak, @cmsbuild, @consuegs, @jfernan2, @mandrenguyen, @perrotta can you please check and sign again.

[bsunanda]

@cmsbuild Please test

[cmsbuild]

+1

Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-09a151/44377/summary.html
COMMIT: 03c7701
CMSSW: CMSSW_15_1_X_2025-02-13-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/47138/44377/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• You potentially added 2 lines to the logs
• Reco comparison results: 2154 differences found in the comparisons
• DQMHistoTests: Total files compared: 49
• DQMHistoTests: Total histograms compared: 3905420
• DQMHistoTests: Total failures: 630
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3904770
• DQMHistoTests: Total skipped: 20
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
• Checked 214 log files, 184 edm output root files, 49 DQM output files
• TriggerResults: found differences in 2 / 47 workflows