PPS fixes in using DBOutputService followup by fabferro · Pull Request #36180 · cms-sw/cmssw

fabferro · 2021-11-19T14:10:40Z

PR description:

Fixes in use of DBOutputService for PPS code.
Followup of PR#36008.

PR validation:

Validated by PPS DB experts. The payloads are produced correctly.

fabferro · 2021-11-19T14:11:21Z

@tvami here is the draft RP

tvami · 2021-11-19T14:14:05Z

@tvami here is the draft RP

Thanks!

cmsbuild · 2021-11-19T14:17:40Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36180/26755

This PR adds an extra 24KB to repository

cmsbuild · 2021-11-19T14:18:05Z

A new Pull Request was created by @fabferro (Fabrizio Ferro) for master.

It involves the following packages:

CondTools/CTPPS (db)
CondTools/Geometry (db)

@cmsbuild, @ggovi, @tvami, @malbouis, @francescobrivio can you please review it and eventually sign? Thanks.
@cvuosalo, @mmusich, @bsunanda this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

tvami · 2021-11-19T14:35:07Z

  edm::Service<cond::service::PoolDBOutputService> poolDbService;
  if (poolDbService.isAvailable()) {
-    poolDbService->writeOneIOV(*pCTPPSPixelAnalysisMask, analysismaskiov_, /*m_record*/ record_);
+    poolDbService->writeOneIOV(analysisMask, analysismaskiov_, /*m_record*/ record_);


Suggested change

poolDbService->writeOneIOV(analysisMask, analysismaskiov_, /*m_record*/ record_);

poolDbService->writeOneIOV(analysisMask, analysismaskiov_, record_);

@tvami here I get from @wpcarvalho (who is testing the package) the following comment:
""It is saving a CTPPSPixelDAQMapping const* object instead of a CTPPSPixelDAQMapping
Removing "&" in
const auto mapping = &es.getData(tokenMapping_);
would fix the issue.""
I'm a bit puzzled now...

tvami · 2021-11-19T14:35:19Z

  edm::Service<cond::service::PoolDBOutputService> poolDbService;
  if (poolDbService.isAvailable()) {
-    poolDbService->writeOneIOV(*pCTPPSPixelDAQMapping, daqmappingiov_, /*m_record*/ record_);
+    poolDbService->writeOneIOV(mapping, daqmappingiov_, /*m_record*/ record_);


Suggested change

poolDbService->writeOneIOV(mapping, daqmappingiov_, /*m_record*/ record_);

poolDbService->writeOneIOV(mapping, daqmappingiov_, record_);

tvami · 2021-11-19T15:00:02Z

+                           &tAR = tokenAlignmentReal_,
+                           &tAM = tokenAlignmentMisaligned_]() -> const CTPPSRPAlignmentCorrectionsData* {
+    if (r == "CTPPSRPAlignmentCorrectionsDataRcd") {
+      return &eS.getData(tAI);


I dont really understand why this couldnt just be

Suggested change

return &eS.getData(tAI);

return &eS.getData(tokenAlignmentIdeal_);

Because, in my understanding, tAI is the captured variable. In fact, if I do your change I get compilation errors.

cmsbuild · 2021-11-22T10:04:46Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36180/26797

This PR adds an extra 24KB to repository

cmsbuild · 2021-11-22T10:05:07Z

Pull request #36180 was updated. @cmsbuild, @ggovi, @tvami, @malbouis, @francescobrivio can you please check and sign again.

tvami · 2021-11-22T17:23:55Z

@cmsbuild , please test

cmsbuild · 2021-11-22T20:32:34Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12ba06/20681/summary.html
COMMIT: 5c96205
CMSSW: CMSSW_12_2_X_2021-11-22-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36180/20681/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: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 42
DQMHistoTests: Total histograms compared: 3247021
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3246999
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 41 files compared)
Checked 177 log files, 37 edm output root files, 42 DQM output files
TriggerResults: no differences found

cmsbuild · 2021-11-26T14:40:16Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36180/26913

This PR adds an extra 28KB to repository

cmsbuild · 2021-11-26T14:40:39Z

Pull request #36180 was updated. @cmsbuild, @ggovi, @tvami, @malbouis, @francescobrivio can you please check and sign again.

tvami · 2021-11-26T23:33:54Z

@fabferro looks good to me, can you please convert it to a full PR?

tvami · 2021-11-26T23:34:05Z

@cmsbuild , please test

cmsbuild · 2021-11-27T10:25:05Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12ba06/20793/summary.html
COMMIT: b0ab699
CMSSW: CMSSW_12_2_X_2021-11-26-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36180/20793/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: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 42
DQMHistoTests: Total histograms compared: 3247873
DQMHistoTests: Total failures: 5
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3247845
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 41 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 177 log files, 37 edm output root files, 42 DQM output files
TriggerResults: no differences found

fabferro · 2021-11-30T15:51:48Z

Hi @tvami, do you think this PR is ok for merging? Thanks.

tvami · 2021-11-30T15:52:55Z

It's fine for me, I wanted @ggovi to have the final word on it tho, let me ask him!

tvami · 2021-12-05T18:02:53Z

+db

I think @ggovi had a look but maybe just forgot to comment?

cmsbuild · 2021-12-05T18:03:15Z

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

perrotta · 2021-12-07T09:48:53Z

+1

fabferro added 2 commits November 9, 2021 11:30

DbOutputService fixes

b4202be

use of lambda expression

485c21c

cmsbuild added this to the CMSSW_12_2_X milestone Nov 19, 2021

cmsbuild added code-checks-pending db-pending orp-pending pending-signatures tests-pending labels Nov 19, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 19, 2021

tvami reviewed Nov 19, 2021

View reviewed changes

clening code

5c96205

cmsbuild added code-checks-pending and removed code-checks-approved labels Nov 22, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 22, 2021

cmsbuild added tests-started and removed tests-pending labels Nov 22, 2021

cmsbuild added tests-approved and removed tests-started labels Nov 22, 2021

cmsbuild added code-checks-pending and removed code-checks-rejected labels Nov 26, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 26, 2021

cmsbuild added tests-started and removed tests-pending labels Nov 26, 2021

cmsbuild added tests-approved and removed tests-started labels Nov 27, 2021

fabferro marked this pull request as ready for review November 27, 2021 11:01

cmsbuild added db-approved fully-signed and removed db-pending pending-signatures labels Dec 5, 2021

smuzaffar modified the milestones: CMSSW_12_2_X, CMSSW_12_3_X Dec 6, 2021

cmsbuild added orp-approved and removed orp-pending labels Dec 7, 2021

cmsbuild merged commit b5b1719 into cms-sw:master Dec 7, 2021

This was referenced Dec 7, 2021

Update pileup scenario in RelVals #36388

Closed

Move to root 6.24 cms-sw/cmsdist#7445

Merged

	poolDbService->writeOneIOV(analysisMask, analysismaskiov_, /m_record/ record_);
	poolDbService->writeOneIOV(analysisMask, analysismaskiov_, record_);

	poolDbService->writeOneIOV(mapping, daqmappingiov_, /m_record/ record_);
	poolDbService->writeOneIOV(mapping, daqmappingiov_, record_);

	return &eS.getData(tAI);
	return &eS.getData(tokenAlignmentIdeal_);

Conversation

fabferro commented Nov 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR description:

PR validation:

Uh oh!

fabferro commented Nov 19, 2021

Uh oh!

tvami commented Nov 19, 2021

Uh oh!

cmsbuild commented Nov 19, 2021

Uh oh!

cmsbuild commented Nov 19, 2021

Uh oh!

tvami Nov 19, 2021

Choose a reason for hiding this comment

Uh oh!

fabferro Nov 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tvami Nov 19, 2021

Choose a reason for hiding this comment

Uh oh!

tvami Nov 19, 2021

Choose a reason for hiding this comment

Uh oh!

fabferro Nov 22, 2021

Choose a reason for hiding this comment

Uh oh!

cmsbuild commented Nov 22, 2021

Uh oh!

cmsbuild commented Nov 22, 2021

Uh oh!

tvami commented Nov 22, 2021

Uh oh!

cmsbuild commented Nov 22, 2021

Comparison Summary

Uh oh!

cmsbuild commented Nov 26, 2021

Uh oh!

cmsbuild commented Nov 26, 2021

Uh oh!

tvami commented Nov 26, 2021

Uh oh!

tvami commented Nov 26, 2021

Uh oh!

cmsbuild commented Nov 27, 2021

Comparison Summary

Uh oh!

fabferro commented Nov 30, 2021

Uh oh!

tvami commented Nov 30, 2021

Uh oh!

tvami commented Dec 5, 2021

Uh oh!

cmsbuild commented Dec 5, 2021

Uh oh!

perrotta commented Dec 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fabferro commented Nov 19, 2021 •

edited

Loading

fabferro Nov 26, 2021 •

edited

Loading