PPS diamond mapping XML for 2023 run

[grzanka]

PR description:

This PR updates the XML mappings for PPS diamond detectors for 2023 run campaign.
The change is due to hardware changes needed to be implemented in PPS to cope with higher trigger rate.
Unfortunately we come up late with this PS, as detectors will be installed end of February 2023.
Backward compatibility is sustained.

PR validation:

This PR was validated with relvals 136.793, 1043 and 1044 (which run PPS reconstruction).
The reconstruction of Run 2 and 2022 data seems not affected.

New XML is parsed properly, I ran cmsRun CalibPPS/ESProducers/test/test_totemDAQMappingESSourceXML_cfg.py to parse XML and print its content on the console.

Also manual tests of reconstruction of 2022 data with 2023 mapping were done, to see if unpacker behaves in expected way.

We do not have proper 2023 data taken with new cabling, so full-scale tests are not possible yet.

[cmsbuild]

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40763/34182

• This PR adds an extra 20KB to repository

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-40763/34182/code-format.patch
  e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40763/34182/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-40763/34183

• This PR adds an extra 20KB to repository

[cmsbuild]

A new Pull Request was created by @grzanka (Leszek Grzanka) for master.

It involves the following packages:

• CalibPPS/ESProducers (alca)
• CondFormats/PPSObjects (alca)
• EventFilter/CTPPSRawToDigi (reconstruction)

@malbouis, @yuanchao, @ChrisMisan, @clacaputo, @cmsbuild, @saumyaphor4252, @tvami, @mandrenguyen, @francescobrivio can you please review it and eventually sign? Thanks.
@fabferro, @tocheng, @Martin-Grunewald, @missirol, @mmusich, @forthommel, @seemasharmafnal this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

[mandrenguyen]

type ctpps

[mandrenguyen]

please test

[francescobrivio]

@cmsbuild please abort

[francescobrivio]

test parameters:

• workflows = 136.793,1043,1044

[tvami]

+alca

• wfs testing Run-2 pass, https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c0a8dd/30676/runTheMatrix-results/136.793_RunDoubleEG2017C/ , https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c0a8dd/30676/runTheMatrix-results/1043.0_RunRawPPS2022B/ , https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c0a8dd/30676/runTheMatrix-results/1044.0_RunRawPPS2022B/
• agreed with @fabferro to do all this from a payload as a follow-up PR

[tvami]

agreed with @fabferro to do all this from a payload as a follow-up PR

should we open an issue so it's not forgotten @fabferro ?

[mandrenguyen]

+reconstruction
This PR is needed for data taking this year. It has no effect at the moment, as the Run3 era would need to be split into 2022 and 2023+, as pointed out in #40763 (comment)
I don't see why we shouldn't merge this now though, and deal with that in a subsequent PR (independent from the payload PR mentioned above)

[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. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

[perrotta]

I don't think it is worth adding this modification without allowing it being able to be included in the normal workflows. A customization of the kind of the one suggested by @makortel in #40763 (comment) and follow ups in the thread.

Perhaps, a less invasive option could be the following:

1. Add ctpps_2023 Modifier, and use that to set the 2023-specific values
2. Rename Era_Run3_cff.py as Era_Run3_2022_cff.py (and the name of the modifier inside it accordingly)
3. Set up the Run3_2023 ModifierChain as Run3_2022.copyAndExclude([ctpps_2022], ctpps_2023)
4. Define a Run3 modifier as a copy of Run3_2023

As such, there will be no need to touch all derived modifiers, in particular not the Phase2 one. And if by chance next year we'll have to define a ctpps_2024, then a Run3_2024 can be easily derived as in (3) here above and the Run3 era can be made identicat to it, without touching anything else.

Workflow specific for a given year can use the corresponding Run3_YYYY era, all other workflows can stay with the generic Run3 era.

@makortel, what do you think?

[grzanka]

I don't think it is worth adding this modification without allowing it being able to be included in the normal workflows.

I didn't get why this modification cannot be used in the normal (i.e. 2023) workflows.
I am not an expert in Eras, but following the discussion on this PR I understood that ctpps_2022 is valid for "2022 runs and onwards". In particular I assume that despite the 2022 name, the ctpps_2022 modifier works as well for 2023 runs.

With commit 6dffcb5 we introduced list of FEDids common to both 2022 and 2023. After that commit there is no need for us to distinguish between 2022 and 2023 using era-based modifier.
See as well my comment: #40763 (comment)

The ctpps_2023 may probably be useful for some other cases (like retrieval of LHCInfo records after switching to the new classes), but here it is not needed to retrieve XML payloads. It may just increase a bit the code clarity (with per-year FEDids), but its not critical.

[makortel]

If the current solution (common list of FEDids) indeed works properly for both 2022 and 2023, I think it is ok (for this PR, at least).

The only nitpick I could make is that from a technical perspective using ctpps_2022 modifier to set the 2022-2023 -specific parameter(s) would be more consistent with how the Era/Modifier system was designed (defaults corresponding to Run 1 or first occurrence of the module, and later changes are applied with a modifier).

[grzanka]

If the current solution (common list of FEDids) indeed works properly for both 2022 and 2023, I think it is ok (for this PR, at least).

Common list of FEDids should work properly both for 2022 and 2023 runs.

The only nitpick I could make is that from a technical perspective using ctpps_2022 modifier to set the 2022-2023 -specific parameter(s) would be more consistent with how the Era/Modifier system was designed (defaults corresponding to Run 1 or first occurrence of the module, and later changes are applied with a modifier).

As for ctpps_2022 modifier I was confused a bit by the discussion here. From the discussion I've deduced that ctpps_2022 should work for 2023 runs, but I've not tested that. Is there a way to check if ctpps_2022 works on 2023 runs on some artificial data, as we don't have data with new cabling ?

The only tests I've done for now was with Run2 and with 2022 data. I've tried to apply PPS data unpacking on 2022 data with 2023 DAQ mapping to check if that produces expected (although unrealistic) results.

[makortel]

As for ctpps_2022 modifier I was confused a bit by the discussion here. From the discussion I've deduced that ctpps_2022 should work for 2023 runs, but I've not tested that. Is there a way to check if ctpps_2022 works on 2023 runs on some artificial data, as we don't have data with new cabling ?

I can only comment that from the technical standpoint, when the job configuration uses Run3 era (or any other era that enables any of the ctpps_{2016,2017,2018} modifiers) there is no functional difference between setting these FEDIds with ctpps_2022 modifier or setting them by default.

Differences can arise on configurations that use Eras that do not enable any of the ctpps_{2016,2017,2018,2022} modifiers. By quick look those would be Run2 eras before 2016 (basically the ones without a specific year, that correspond to various parts of 2015), and Phase2 eras.

After a bit of poking it seems to me that the ctppsDiamondRawToDigi module gets added to the high-level Task(a) with ctpps modifier, which is enabled in Run2_2016 era, but not removed in Phase2 era (but I could have missed something). So one could at least ask the question of how this code should behave in Phase2 workflows (e.g. I don't know if it gets effectively disabled in some way).

[perrotta]

So, given the answers recieved to my comment, I'm realizing that maybe I misunderstood how this PR works: @grzanka can you confirm that it can already pick either the 2022 mapping or the 2023 one (according ro the EventRange I imagine) without the need to customize it via eras or midifiers? What about MC?

[grzanka]

So, given the answers recieved to my comment, I'm realizing that maybe I misunderstood how this PR works: @grzanka can you confirm that it can already pick either the 2022 mapping or the 2023 one (according ro the EventRange I imagine)

Manual testing with empty source producing events witch arbitrary run numbers shows that run selection mechanism works without the need of era modifiers.I was using using an EDAnalyzer which prints the mapping:
cmsRun CalibPPS/ESProducers/test/test_totemDAQMappingESSourceXML_cfg.py

without the need to customize it via eras or midifiers?

We don't have data yet for 2023 to test it with proper 2023-run number.
If you have any idea how to make some artificial test with 2023 run number I could give it another try.
According to what @makortel this should work.

What about MC?

This shouldn't affect MC runs. As for status of PPS simulation, I will cross-check with experts.

[grzanka]

I can only comment that from the technical standpoint, when the job configuration uses Run3 era (or any other era that enables any of the ctpps_{2016,2017,2018} modifiers) there is no functional difference between setting these FEDIds with ctpps_2022 modifier or setting them by default.

I am a bit confused by statement that:

• Run3 era (or any other era that enables any of the ctpps_{2016,2017,2018} modifiers)

Does it imply that Run3 era enables ctpps_{2016,2017,2018} modifiers ?

From the code here
https://github.com/cms-sw/cmssw/blob/master/Configuration/Eras/python/Era_Run3_cff.py
I see that Run3 era imports ctpps_2018 and ctpps_2022 modifiers. Then it excludes ctpps_2018 and includes ctpps_2022 using cms.ModifierChain

[makortel]

I can only comment that from the technical standpoint, when the job configuration uses Run3 era (or any other era that enables any of the ctpps_{2016,2017,2018} modifiers) there is no functional difference between setting these FEDIds with ctpps_2022 modifier or setting them by default.

I am a bit confused by statement that:

• Run3 era (or any other era that enables any of the ctpps_{2016,2017,2018} modifiers)

Does it imply that Run3 era enables ctpps_{2016,2017,2018} modifiers ?

No. I meant generally any era that enables one of the ctpps_2016, ctpps_2017, ctpps_2018, or ctpps_2022 modifiers. By quick look these would be Run2_2016, Run2_2017, Run2_2018, and Run3, and Eras that are derived from these (without excluding the ctpps_* modifier).

[perrotta]

+1

• As discussed in the ORP meeting on Feb 21 (even if none from PPS intervened...) let have this merged even if it is clear that the configuration is rather messy, and certainly not suitable for MC. The conclusion was that "it doesn't hurt.", and @grzanka assured here above that "the run selection mechanism works without the need of era modifiers"
• AlCa/DB: it is okay for now for the XML solution but then we should move to CondDB later on.

[mandrenguyen]

@grzanka Please prepare a backport to 13_0_X

[grzanka]

@grzanka Please prepare a backport to 13_0_X

OK, I will do it.

[grzanka]

@grzanka Please prepare a backport to 13_0_X

Backport is ready here #40843