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

Update 141X data GTs in autoCond #46824

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

perrotta
Copy link
Contributor

@perrotta perrotta commented Nov 28, 2024

Update online GTs in autoCond after the end of the 2024 data taking (to be used for tests and development):

Also the offline data GT needed to be fixed:

Moreover, to avoid crashes in the unit tests because of the removed (now unused) records in the HLT GT, the commit 6198154 comments out the scal_dqm_sourceclient-live_cfg.py test from the ones run in the unit tests of DQM/Integration: as correctly reported in a comment here below, the SCAL FED has been removed from DAQ since the end of Run 2.

PR validation:

The removal of the not-needed records from the HLT GT was verified online as documented in CMSALCAFAST-94
Rely on these PR tests for a wider coverage

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 28, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • Configuration/AlCa (alca)

@atpathak, @cmsbuild, @consuegs, @perrotta can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @fabiocos, @missirol, @mmusich, @rsreds, @tocheng, @yuanchao 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
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: UnitTests
Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1f6165/43137/summary.html
COMMIT: 6198154
CMSSW: CMSSW_15_0_X_2024-11-28-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/46824/43137/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found 1 errors in the following unit tests:

---> test TestDQMOnlineClient-scal_dqm_sourceclient had ERRORS

Comparison Summary

Summary:

  • You potentially added 1 lines to the logs
  • Reco comparison results: 6 differences found in the comparisons
  • DQMHistoTests: Total files compared: 46
  • DQMHistoTests: Total histograms compared: 3484682
  • DQMHistoTests: Total failures: 438
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3484224
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 45 files compared)
  • Checked 202 log files, 172 edm output root files, 46 DQM output files
  • TriggerResults: no differences found

@perrotta
Copy link
Contributor Author

please test with #46742

@perrotta
Copy link
Contributor Author

perrotta commented Nov 28, 2024

abort
(#46742 has nothing to do with the removal of the unused records...)

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46824/42833

@cmsbuild
Copy link
Contributor

Pull request #46824 was updated. @antoniovagnerini, @atpathak, @cmsbuild, @consuegs, @perrotta, @rseidita can you please check and sign again.

@perrotta
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: UnitTests
Size: This PR adds an extra 24KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1f6165/43146/summary.html
COMMIT: b0b9c55
CMSSW: CMSSW_15_0_X_2024-11-28-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/46824/43146/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found 1 errors in the following unit tests:

---> test TestDQMOnlineClient-scal_dqm_sourceclient had ERRORS

Comparison Summary

Summary:

@@ -76,7 +76,7 @@
process.dqmmodules = cms.Sequence(process.dqmEnv + process.dqmSaver )#+ process.dqmSaverPB)
process.evfDQMmodulesPath = cms.Path(
process.l1GtUnpack*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, just a couple of quick comments:

  • from the exception message [1] log here , actually (also) l1GtUnpack needs to be changed.
  • the SCAL FED has been removed from DAQ since the end of Run 2. Do we still need to support this client? The fact that CMSALCAFAST-94 succeeded without these changes is proof that this client is not run anymore in live data-taking.

[1]

----- Begin Fatal Exception 29-Nov-2024 00:46:25 CET-----------------------
An exception of category 'NoRecord' occurred while
   [0] Processing  Event run: 355380 lumi: 19 event: 26716348 stream: 0
   [1] Running path 'evfDQMmodulesPath'
   [2] Calling method for module L1GlobalTriggerRawToDigi/'l1GtUnpack'
Exception Message:
No "L1MuTriggerScalesRcd" record found in the EventSetup.

 Please add an ESSource or ESProducer that delivers such a record.
----- End Fatal Exception -------------------------------------------------

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @mmusich
Indeed, removing that test at all would be a much simpler solution: if you also support it, I'll do so. (Now I am traveling, I will do it as soon as I will be able to)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removing that test at all would be a much simpler solution:

rather than removing the test, I would suggest to remove completely the configuration file of the client.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that would make sense. But I'd let it be done in the proper way by @cms-sw/l1-l2 (or by whoever else wants to do it).
For the sake of just having the updated GTs running without crashes in the CMSSW unit tests system let just comment out here the obsolete one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cms-sw/dqm-l2 can you comment on a possible removal of the client from the release already in this PR?

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Pull request #46824 was updated. @antoniovagnerini, @atpathak, @cmsbuild, @consuegs, @perrotta, @rseidita can you please check and sign again.

@perrotta
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

+1

Size: This PR adds an extra 20KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1f6165/43164/summary.html
COMMIT: 23b9148
CMSSW: CMSSW_15_0_X_2024-11-29-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/46824/43164/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 6 differences found in the comparisons
  • DQMHistoTests: Total files compared: 46
  • DQMHistoTests: Total histograms compared: 3484682
  • DQMHistoTests: Total failures: 491
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3484171
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 45 files compared)
  • Checked 202 log files, 172 edm output root files, 46 DQM output files
  • TriggerResults: found differences in 1 / 44 workflows

@perrotta
Copy link
Contributor Author

+alca

  • No changes were expected because of the GT updates (HCAL response corrections in the offline GT were only updated with a small fix for eras 2024C and 2024E; all other GT updates are kind of technical), and no changes were observed in the test outputs

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.

3 participants