Change PPS geometry ES producer to read preprocessed geometry from DB #33851

wpcarvalho · 2021-05-26T16:36:34Z

PR description:

The goal of this PR is to make the ES producer CTPPSGeometryESModule.cc able to read the PPS reco geometry from conditions data stored in the database. Currently, PPS geometry is read directly from XML files.

No changes are expected in the output.
We plan a future PR that will change the default reco sequence to consume conditions data from DB.

@fabferro @jan-kaspar

PR validation:

The following validation tests have been performed:

Comparison of pot and sensor positions obtained from XML (current producer) and from DB (changed producer). Perfect agreement is found.
Reco workflow. Local tests have been run to compare the current and the changed versions of the producer. Nearly perfect agreement has been observed, as can be seen from a sample of plots shown below. Only a few very tiny differences (probably at the round level) among hundreds of histograms. See the Mean for theta_x and theta_y histograms in the bottom canvas (top row = baseline ; bottom row = changed producer) as the only visible differences.

The tests with the changed producer used conditions data from a local sqlite file and also the same data uploaded to the Prep database as a test tag: https://cms-conddb.cern.ch/cmsDbBrowser/list/Prep/tags/PPS_RecoGeometry_test_v1.

if this PR is a backport please specify the original PR and why you need to backport that PR:

Not a backport.

…y from DB

cmsbuild · 2021-05-26T16:41:13Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33851/22874

This PR adds an extra 24KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-checks:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33851/22874/code-checks.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33851/22874/code-checks.patch | patch -p1
You can also run scram build code-checks to apply code checks directly
code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33851/22874/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33851/22874/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2021-05-26T18:26:02Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33851/22876

This PR adds an extra 24KB to repository

cmsbuild · 2021-05-26T18:26:29Z

A new Pull Request was created by @wpcarvalho (Wagner Carvalho) for master.

It involves the following packages:

Geometry/Records
Geometry/VeryForwardGeometryBuilder

@civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild can you please review it and eventually sign? Thanks.
@forthommel, @jan-kaspar, @fabiocos this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

civanch · 2021-05-26T20:24:21Z

please test

cmsbuild · 2021-05-26T23:14:26Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3b7b9e/15335/summary.html
COMMIT: 8cf6b07
CMSSW: CMSSW_12_0_X_2021-05-26-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33851/15335/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

The workflows 140.53 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

Summary:

No significant changes to the logs found
Reco comparison results: 1268 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2650486
DQMHistoTests: Total failures: 3681
DQMHistoTests: Total nulls: 19
DQMHistoTests: Total successes: 2646764
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 45.703 KiB( 36 files compared)
DQMHistoSizes: changed ( 140.53 ): 44.531 KiB Hcal/DigiRunHarvesting
DQMHistoSizes: changed ( 140.53 ): 1.172 KiB RPC/DCSInfo
Checked 155 log files, 37 edm output root files, 37 DQM output files
TriggerResults: no differences found

cvuosalo · 2021-05-27T19:32:09Z

Geometry/VeryForwardGeometryBuilder/src/DetGeomDesc.cc

+    : m_name(item.name_),
+      m_copy(item.copy_),
+      m_isDD4hep(true),
+      m_params(item.params_),  // default unit from DD4hep (cm)


Default unit for DD4hep is not cm. Delete "(cm)" from the comment.

cvuosalo · 2021-05-27T19:32:37Z

Geometry/VeryForwardGeometryBuilder/src/DetGeomDesc.cc

+    : m_name(item.name_),
+      m_copy(item.copy_),
+      m_isDD4hep(true),
+      m_params(item.params_),  // default unit from DD4hep (cm)


Suggested change

m_params(item.params_), // default unit from DD4hep (cm)

m_params(item.params_), // default unit from DD4hep

cvuosalo · 2021-05-27T19:33:07Z

Geometry/VeryForwardGeometryBuilder/src/DetGeomDesc.cc

+      m_params(item.params_),  // default unit from DD4hep (cm)
+      m_sensorType(item.sensorType_),
+      m_geographicalID(item.geographicalID_),
+      m_z(item.z_)  // converted from cm (DD4hep) to mm


Suggested change

m_z(item.z_) // converted from cm (DD4hep) to mm

m_z(item.z_) // converted from DD4hep units to mm

cmsbuild · 2021-05-27T20:21:57Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33851/22906

This PR adds an extra 16KB to repository

cmsbuild · 2021-05-27T20:22:23Z

Pull request #33851 was updated. @civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild can you please check and sign again.

cvuosalo · 2021-05-27T20:26:52Z

+1

cmsbuild · 2021-05-27T20:27:13Z

This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

cvuosalo · 2021-05-27T20:27:26Z

@cmsbuild please test

cmsbuild · 2021-05-27T23:32:34Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3b7b9e/15372/summary.html
COMMIT: 4f8c283
CMSSW: CMSSW_12_0_X_2021-05-27-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33851/15372/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: 8 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2650486
DQMHistoTests: Total failures: 12
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 2650451
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 36 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 155 log files, 37 edm output root files, 37 DQM output files
TriggerResults: no differences found

qliphy · 2021-05-28T03:05:40Z

+1

Make changes in PPS geometry ES producer to read preprocessed geometr…

9b3c3bb

…y from DB

cmsbuild added this to the CMSSW_12_0_X milestone May 26, 2021

cmsbuild added code-checks-pending geometry-pending orp-pending pending-signatures tests-pending labels May 26, 2021

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

Apply patches

8cf6b07

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

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

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

cmsbuild added tests-approved and removed tests-started labels May 26, 2021

cvuosalo reviewed May 27, 2021

View reviewed changes

Correct a couple of comments

4f8c283

cmsbuild added code-checks-pending and removed code-checks-approved tests-approved labels May 27, 2021

cmsbuild added the tests-pending label May 27, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels May 27, 2021

cmsbuild added fully-signed geometry-approved tests-started and removed geometry-pending pending-signatures tests-pending labels May 27, 2021

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

cmsbuild added orp-approved and removed orp-pending labels May 28, 2021

cmsbuild merged commit 628c74d into cms-sw:master May 28, 2021

wpcarvalho mentioned this pull request Jul 19, 2021

Switch PPS reco to use preprocessed geometry from DB #34549

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change PPS geometry ES producer to read preprocessed geometry from DB #33851

Change PPS geometry ES producer to read preprocessed geometry from DB #33851

wpcarvalho commented May 26, 2021

cmsbuild commented May 26, 2021

cmsbuild commented May 26, 2021

cmsbuild commented May 26, 2021

civanch commented May 26, 2021

cmsbuild commented May 26, 2021

cvuosalo May 27, 2021

cvuosalo May 27, 2021

wpcarvalho May 27, 2021

cvuosalo May 27, 2021

wpcarvalho May 27, 2021

cmsbuild commented May 27, 2021

cmsbuild commented May 27, 2021

cvuosalo commented May 27, 2021

cmsbuild commented May 27, 2021

cvuosalo commented May 27, 2021

cmsbuild commented May 27, 2021

qliphy commented May 28, 2021

	m_params(item.params_), // default unit from DD4hep (cm)
	m_params(item.params_), // default unit from DD4hep

	m_z(item.z_) // converted from cm (DD4hep) to mm
	m_z(item.z_) // converted from DD4hep units to mm

Change PPS geometry ES producer to read preprocessed geometry from DB #33851

Change PPS geometry ES producer to read preprocessed geometry from DB #33851

Conversation

wpcarvalho commented May 26, 2021

PR description:

PR validation:

if this PR is a backport please specify the original PR and why you need to backport that PR:

cmsbuild commented May 26, 2021

cmsbuild commented May 26, 2021

cmsbuild commented May 26, 2021

civanch commented May 26, 2021

cmsbuild commented May 26, 2021

Comparison Summary

cvuosalo May 27, 2021

Choose a reason for hiding this comment

cvuosalo May 27, 2021

Choose a reason for hiding this comment

wpcarvalho May 27, 2021

Choose a reason for hiding this comment

cvuosalo May 27, 2021

Choose a reason for hiding this comment

wpcarvalho May 27, 2021

Choose a reason for hiding this comment

cmsbuild commented May 27, 2021

cmsbuild commented May 27, 2021

cvuosalo commented May 27, 2021

cmsbuild commented May 27, 2021

cvuosalo commented May 27, 2021

cmsbuild commented May 27, 2021

Comparison Summary

qliphy commented May 28, 2021