PPS fixes in using DBOutputService followup #36180

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

CondTools/CTPPS/src/WriteCTPPSPixelAnalysisMask.cc

  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

CondTools/CTPPS/src/WriteCTPPSPixelDAQMapping.cc

  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

CondTools/CTPPS/src/CTPPSRPAlignmentInfoAnalyzer.cc

+                           &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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PPS fixes in using DBOutputService followup #36180

PPS fixes in using DBOutputService followup #36180

fabferro commented Nov 19, 2021 •

edited

fabferro commented Nov 19, 2021

tvami commented Nov 19, 2021

cmsbuild commented Nov 19, 2021

cmsbuild commented Nov 19, 2021

tvami Nov 19, 2021

fabferro Nov 26, 2021 •

edited

tvami Nov 19, 2021

tvami Nov 19, 2021

fabferro Nov 22, 2021

cmsbuild commented Nov 22, 2021

cmsbuild commented Nov 22, 2021

tvami commented Nov 22, 2021

cmsbuild commented Nov 22, 2021

cmsbuild commented Nov 26, 2021

cmsbuild commented Nov 26, 2021

tvami commented Nov 26, 2021

tvami commented Nov 26, 2021

cmsbuild commented Nov 27, 2021

fabferro commented Nov 30, 2021

tvami commented Nov 30, 2021

tvami commented Dec 5, 2021

cmsbuild commented Dec 5, 2021

perrotta commented Dec 7, 2021

	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_);

PPS fixes in using DBOutputService followup #36180

PPS fixes in using DBOutputService followup #36180

Conversation

fabferro commented Nov 19, 2021 • edited

PR description:

PR validation:

fabferro commented Nov 19, 2021

tvami commented Nov 19, 2021

cmsbuild commented Nov 19, 2021

cmsbuild commented Nov 19, 2021

tvami Nov 19, 2021

Choose a reason for hiding this comment

fabferro Nov 26, 2021 • edited

Choose a reason for hiding this comment

tvami Nov 19, 2021

Choose a reason for hiding this comment

tvami Nov 19, 2021

Choose a reason for hiding this comment

fabferro Nov 22, 2021

Choose a reason for hiding this comment

cmsbuild commented Nov 22, 2021

cmsbuild commented Nov 22, 2021

tvami commented Nov 22, 2021

cmsbuild commented Nov 22, 2021

Comparison Summary

cmsbuild commented Nov 26, 2021

cmsbuild commented Nov 26, 2021

tvami commented Nov 26, 2021

tvami commented Nov 26, 2021

cmsbuild commented Nov 27, 2021

Comparison Summary

fabferro commented Nov 30, 2021

tvami commented Nov 30, 2021

tvami commented Dec 5, 2021

cmsbuild commented Dec 5, 2021

perrotta commented Dec 7, 2021

fabferro commented Nov 19, 2021 •

edited

fabferro Nov 26, 2021 •

edited