[HGC trigger] Replace exception with warning for missing wafer in module mapping

[jbsauvan]

PR description:

Replace exception, when a wafer is not found in the trigger module mapping, by a warning.

Mappings are now declared as mutable in order to cache the missing modules and avoid logging a warning multiple times for the same wafer. But I'm not sure whether mutable members are authorized.

PR validation:

Tested with D59 (27834.0 workflow), which currently throws an exception.

[cmsbuild]

The code-checks are being triggered in jenkins.

[silviodonato]

please test

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30541/16766

• This PR adds an extra 16KB to repository

[cmsbuild]

The tests are being triggered in jenkins.

• CMSSW_11_2_X_2020-07-04-1100/slc7_amd64_gcc820: https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/7695/console Started: 2020/07/04 21:59

[cmsbuild]

A new Pull Request was created by @jbsauvan (Jean-Baptiste Sauvan) for master.

It involves the following packages:

L1Trigger/L1THGCal

@cmsbuild, @rekovic, @benkrikler, @kpedro88 can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @amarini, @lgray this is something you requested to watch as well.
@silviodonato, @dpiparo you are the release manager for this.

cms-bot commands are listed here

[silviodonato]

please test

[kpedro88]

@jbsauvan this particular use of mutable does not appear thread-safe. Because the geometry object is accessible from multiple threads, updates to its member variables need to be done in an atomic way. However, the usual approach (making a copy, updating that, then trying to swap the copy for the original, which is an atomic operation) might be memory-prohibitive in this case.
@makortel @Dr15Jones is there another way to accomplish this?

[cmsbuild]

+1
Tested at: 561354d
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-74bb68/7695/summary.html
CMSSW: CMSSW_11_2_X_2020-07-04-1100
SCRAM_ARCH: slc7_amd64_gcc820

[cmsbuild]

Comparison job queued.

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-74bb68/7695/summary.html

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 2 differences found in the comparisons
• DQMHistoTests: Total files compared: 37
• DQMHistoTests: Total histograms compared: 2787364
• DQMHistoTests: Total failures: 5
• DQMHistoTests: Total nulls: 1
• DQMHistoTests: Total successes: 2787308
• DQMHistoTests: Total skipped: 50
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.004 KiB( 36 files compared)
• DQMHistoSizes: changed ( 10224.0 ): 0.004 KiB MessageLogger/Warnings
• Checked 154 log files, 17 edm output root files, 37 DQM output files

[makortel]

The mutable is certainly unsafe here. Another consideration is that the ES (and ED) products are required to be "logically const" after being produced, e.g. caching is allowed but there must not be any visible difference whether something is in the cache or not. Here it is not immediately clear to me if all the functions using module_to_wafers_ behave similarly in the presence of "disconnected modules" in the mapping.

Assuming the behavior follows the requirement, TBB does provide tbb::concurrent_unordered_map that permits "concurrent traversal and insertion".

An alternative idea would be to use tbb::concurrent_set to record the packLayerWaferId() values for which the warning has been issued already. A downside is that for "missing wafers" two searches need to be made.

@Dr15Jones do you have better ideas?

[jbsauvan]

The solution adding a concurrent_set is probably the best. In general there are no missing wafers, it would just happen in the case of workflows with HGCAL geometries in development phase. And even in this case the number of missing wafers should be small compared to the total number of wafers.
I'll implement that.

[cmsbuild]

+1
Tested at: 73bd56f
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-74bb68/7698/summary.html
CMSSW: CMSSW_11_2_X_2020-07-05-0000
SCRAM_ARCH: slc7_amd64_gcc820

[cmsbuild]

Comparison job queued.

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-74bb68/7698/summary.html

@slava77 comparisons for the following workflows were not done due to missing matrix map:

• /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-74bb68/27834.0_TTbar_14TeV+TTbar_14TeV_TuneCP5_2026D59_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D59+RecoFullGlobal_2026D59+HARVESTFullGlobal_2026D59

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 0 differences found in the comparisons
• DQMHistoTests: Total files compared: 37
• DQMHistoTests: Total histograms compared: 2787364
• DQMHistoTests: Total failures: 1
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2787313
• DQMHistoTests: Total skipped: 50
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
• Checked 154 log files, 17 edm output root files, 37 DQM output files

[silviodonato]

urgent
for CMSSW_11_2_0_pre2

[kpedro88]

+upgrade

[rekovic]

+1

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo (and backports should be raised in the release meeting by the corresponding L2)

[silviodonato]

+1

[silviodonato]

@jbsauvan could you prepare a backport to 11_1_X? Thanks

[jbsauvan]

Backport done in #30596