Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PPS diamond mapping fix for 2023 run #41187

Merged
merged 2 commits into from
Mar 28, 2023

Conversation

grzanka
Copy link
Contributor

@grzanka grzanka commented Mar 25, 2023

PR description:

This PR updates the XML mappings for PPS diamond detectors for 2023 run campaign.
The change follows recently discovered hardware issue, related to bad fiber input connector in the NEW OptoRX installed to cope with the new high trigger rate foreseen. Backward compatibility is sustained.

During tests of this PR we discovered as well that on of the parameters in the PPS DQM module was not marked as untracked.
This was probably a leftover from a migration campaign done in #38907
Appropriate change comes as well with this PR.

See as well related #40763

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.
We ran the tests of unpacker on dataset(run 364968) taken after hardware intervention. The raw2digi step produced expected results.
We do not have proper 2023 data taken with new cabling, so full-scale reconstruction tests are not possible yet.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41187/34872

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • CondFormats/PPSObjects (alca)
  • DQM/CTPPS (dqm)

@pmandrik, @emanueleusai, @tvami, @cmsbuild, @saumyaphor4252, @syuvivida, @rvenditti, @micsucmed, @francescobrivio can you please review it and eventually sign? Thanks.
@tocheng, @fabferro, @mmusich, @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

@francescobrivio
Copy link
Contributor

type ctpps

@francescobrivio
Copy link
Contributor

test parameters:

  • workflows = 136.793,1043,1044

@cmsbuild cmsbuild added the ctpps label Mar 25, 2023
@francescobrivio
Copy link
Contributor

francescobrivio commented Mar 25, 2023

Thanks @grzanka for this fix!
Let me just remind you that, as discussed in #40763 (comment) we agreed that for now it's ok to have the xml solution, but it would be preferred to have it implemented in the GT, the corresponding issue to track this update is #40967

We will also need a 13_0_X backport if this is intended for 2023 data-taking.

@francescobrivio
Copy link
Contributor

@cmsbuild please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ebc5d0/31592/summary.html
COMMIT: bb5cb25
CMSSW: CMSSW_13_1_X_2023-03-25-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/41187/31592/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially removed 3 lines from the logs
  • Reco comparison results: 1 differences found in the comparisons
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3554888
  • DQMHistoTests: Total failures: 3
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3554863
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
  • Checked 221 log files, 166 edm output root files, 51 DQM output files
  • TriggerResults: no differences found

@grzanka
Copy link
Contributor Author

grzanka commented Mar 26, 2023

Thanks @grzanka for this fix! Let me just remind you that, as discussed in #40763 (comment) we agreed that for now it's ok to have the xml solution, but it would be preferred to have it implemented in the GT, the corresponding issue to track this update is #40967

Thanks for reminder, we already planned a campaign to move the records to GT, this should be accomplished during summer 2023.

We will also need a 13_0_X backport if this is intended for 2023 data-taking.

Here is the backport: #41188

@francescobrivio
Copy link
Contributor

+1

@perrotta
Copy link
Contributor

type bug-fix

@tvami
Copy link
Contributor

tvami commented Mar 28, 2023

@cms-sw/dqm-l2 do you have any comments?

@tvami
Copy link
Contributor

tvami commented Mar 28, 2023

urgent

  • the backport is urgent

@emanueleusai
Copy link
Member

+1

@cmsbuild
Copy link
Contributor

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
Copy link
Contributor

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants