PPS geometry for Run3 #34499

fabferro · 2021-07-15T13:20:51Z

PR description:

This PR implements the description of Run3 PPS detectors (tracking and timing) at the status of art of current knowledge (2021/Run3 geometry for PPS was till now a replica of Run2 geometry).
Two geometries have been implemented: one to be used in Simulation workflows and one in Reconstruction's.
This is a peculiarity of PPS that we need to conserve and that arises from the specific features of how we deal with the connection with LHC machine in the two workflows.
Most of the files can be used for both geometries, when it was not possible, the involved files have been placed in Reco/Simu subdirs.
The geometry described in dict2021Geometry.py P5 contains the Simu version, the one described in geometryRPFromDD_2021_cfi.py contains the Reco version.
We have placed the new files in directories marked with 2021 flags trying to comply as much as possible to the Geometry naming conventions, provided that we also had to deal with two different detectors placed in four different pots, and a strict observations of the rules was not considered efficient nor clear enough.
Files needed in Run2 geometries have not been touched, to keep a full back-compatibility.
The tools to produce .db files for 2021 geometry has been update3d as well.

PR validation:

The geometries have been tested with a full (private) Simu+Reco chain which loads the two geometries, checking that we obtain compatible results.
Same results are obtained reading directly the xml's and reading the .db files produced with the official tools.
Tests with Run2 data has also been done: Run2 data processing is not affected by the PR.
The detectors are still being built thus no tests on real data could be performed.

cmsbuild · 2021-07-15T13:28:03Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34499/23978

This PR adds an extra 84KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Configuration/Geometry/python/dict2021Geometry.py modified in PR(s): Run 2, Run3, and Phase2 tracker: adding T27 (geometry with bricked pixels) and support for storing bricked pixel information #34120

cmsbuild · 2021-07-15T13:28:28Z

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

It involves the following packages:

CondTools/Geometry (db)
Configuration/Geometry (geometry, upgrade)
Geometry/CMSCommonData (geometry, upgrade)
Geometry/VeryForwardData (geometry)
Geometry/VeryForwardGeometry (geometry)

@civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @kpedro88, @cmsbuild, @srimanob, @mdhildreth, @ggovi can you please review it and eventually sign? Thanks.
@vargasa, @cvuosalo, @jan-kaspar, @Martin-Grunewald, @bsunanda, @mmusich, @fabiocos, @slomeo this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy, @perrotta you are the release manager for this.

cms-bot commands are listed here

fabferro · 2021-07-15T13:34:49Z

FYI @wpcarvalho

civanch · 2021-07-15T13:38:16Z

please test

cmsbuild · 2021-07-15T17:26:23Z

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c2fead/16851/summary.html
COMMIT: c293caf
CMSSW: CMSSW_12_0_X_2021-07-15-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/34499/16851/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test test2021Geometry had ERRORS

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 38
DQMHistoTests: Total histograms compared: 2786302
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2786279
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
Checked 160 log files, 37 edm output root files, 38 DQM output files
TriggerResults: no differences found

cvuosalo · 2021-07-15T18:29:07Z

@fabferro I don't understand the title of this PR. pre4 is done. It was made last week. This PR might be able to get into pre5, but pre4 is past.

cvuosalo · 2021-07-15T18:30:28Z

Geometry/VeryForwardGeometry/python/dd4hep/v2

@@ -0,0 +1,18 @@
+import FWCore.ParameterSet.Config as cms


This file does not have a correct name.

It should be a dir actually. Something went wrong... I fixed it.

cvuosalo · 2021-07-15T18:35:01Z

Configuration/Geometry/python/generate2021Geometry.py needs to be run, and the files it changes need to be included in this PR. Also Configuration/Geometry/README.md needs to be updated.

cmsbuild · 2021-07-21T10:02:39Z

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

qliphy · 2021-07-21T12:39:19Z

+1

qliphy · 2021-07-22T08:04:43Z

@fabferro
We are getting many IB errors after this PR getting merged. For example:
https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc900/CMSSW_12_0_X_2021-07-21-2300/pyRelValMatrixLogs/run/11650.911_ZMM_14+2021_DD4hep+ZMM_14TeV_TuneCP5_GenSim+Digi+Reco+HARVEST+ALCA/step1_ZMM_14+2021_DD4hep+ZMM_14TeV_TuneCP5_GenSim+Digi+Reco+HARVEST+ALCA.log#/40-40
edm::FileInPath unable to find file Geometry/VeryForwardData/data/RP_Vertical_Device/2021/v1/RP_Vertical_Device.xml anywhere in the search path.

Please make a PR to upload those xml files to the repository the sooner the better:
https://github.com/cms-data/Geometry-VeryForwardData

e.g. the above xml should be put under a directory RP_Vertical_Device/2021/v1 in the repository

fabferro · 2021-07-22T08:31:31Z

@fabferro
We are getting many IB errors after this PR getting merged. For example:
https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc900/CMSSW_12_0_X_2021-07-21-2300/pyRelValMatrixLogs/run/11650.911_ZMM_14+2021_DD4hep+ZMM_14TeV_TuneCP5_GenSim+Digi+Reco+HARVEST+ALCA/step1_ZMM_14+2021_DD4hep+ZMM_14TeV_TuneCP5_GenSim+Digi+Reco+HARVEST+ALCA.log#/40-40
edm::FileInPath unable to find file Geometry/VeryForwardData/data/RP_Vertical_Device/2021/v1/RP_Vertical_Device.xml anywhere in the search path.

Please make a PR to upload those xml files to the repository the sooner the better:
https://github.com/cms-data/Geometry-VeryForwardData

e.g. the above xml should be put under a directory RP_Vertical_Device/2021/v1 in the repository

Working on it...

smuzaffar · 2021-07-22T08:39:01Z

Currently all the xml files are in cmssw/Geometry/VeryForwardData . If there is not strong reason to keep these files in cmssw then pls move cmssw/Geometry/VeryForwardData/data to https://github.com/cms-data/Geometry-VeryForwardData

qliphy · 2021-07-22T08:46:17Z

Moreover, @fabferro as you moved
[1] Geometry/VeryForwardData/data/RP_Vertical_Device/2021/v1/RP_Vertical_Device.xml
to
[2] Geometry/VeryForwardData/data/RP_Vertical_Device/2021/Simu/v1/RP_Vertical_Device.xml

this brings errors to complain:
edm::FileInPath unable to find file Geometry/VeryForwardData/data/RP_Vertical_Device/2021/v1/RP_Vertical_Device.xml anywhere in the search path.
in e.g.
https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc900/CMSSW_12_0_X_2021-07-21-2300/unitTestLogs/DetectorDescription/DDCMS#/547-547
and
https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc900/CMSSW_12_0_X_2021-07-21-2300/unitTestLogs/Geometry/TrackerGeometryBuilder#/73-73

Should we keep [1] instead?

fabferro · 2021-07-22T08:51:23Z

@fabferro
We are getting many IB errors after this PR getting merged. For example:
https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc900/CMSSW_12_0_X_2021-07-21-2300/pyRelValMatrixLogs/run/11650.911_ZMM_14+2021_DD4hep+ZMM_14TeV_TuneCP5_GenSim+Digi+Reco+HARVEST+ALCA/step1_ZMM_14+2021_DD4hep+ZMM_14TeV_TuneCP5_GenSim+Digi+Reco+HARVEST+ALCA.log#/40-40
edm::FileInPath unable to find file Geometry/VeryForwardData/data/RP_Vertical_Device/2021/v1/RP_Vertical_Device.xml anywhere in the search path.
Please make a PR to upload those xml files to the repository the sooner the better:
https://github.com/cms-data/Geometry-VeryForwardData
e.g. the above xml should be put under a directory RP_Vertical_Device/2021/v1 in the repository

Working on it...

@qliphy It seems I miss the permission to write to the repository.

qliphy · 2021-07-22T08:52:27Z

@qliphy It seems I miss the permission to write to the repository.

@fabferro You can make a PR similar as to cms-sw

fabferro · 2021-07-22T09:03:59Z

Moreover, @fabferro as you moved
[1] Geometry/VeryForwardData/data/RP_Vertical_Device/2021/v1/RP_Vertical_Device.xml
to
[2] Geometry/VeryForwardData/data/RP_Vertical_Device/2021/Simu/v1/RP_Vertical_Device.xml

this brings errors to complain:
edm::FileInPath unable to find file Geometry/VeryForwardData/data/RP_Vertical_Device/2021/v1/RP_Vertical_Device.xml anywhere in the search path.
in e.g.
https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc900/CMSSW_12_0_X_2021-07-21-2300/unitTestLogs/DetectorDescription/DDCMS#/547-547
and
https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc900/CMSSW_12_0_X_2021-07-21-2300/unitTestLogs/Geometry/TrackerGeometryBuilder#/73-73

Should we keep [1] instead?

No, we should have both. One is used for P4 and one for P5. The version used for P4 can't be deleted until P5 becomes the default and P4 deprecated.

qliphy · 2021-07-22T09:10:06Z

No, we should have both. One is used for P4 and one for P5. The version used for P4 can't be deleted until P5 becomes the default and P4 deprecated.

Yes, I know. My question is that you have already moved
Geometry/VeryForwardData/data/RP_Vertical_Device/2021/v1/RP_Vertical_Device.xml
to
Geometry/VeryForwardData/data/RP_Vertical_Device/2021/Simu/v1/RP_Vertical_Device.xml

but there are other codes still requiring
Geometry/VeryForwardData/data/RP_Vertical_Device/2021/v1/RP_Vertical_Device.xml
so should we keep it? or should we touch those other codes?

fabferro · 2021-07-22T09:16:30Z

No, we should have both. One is used for P4 and one for P5. The version used for P4 can't be deleted until P5 becomes the default and P4 deprecated.

Yes, I know. My question is that you have already moved
Geometry/VeryForwardData/data/RP_Vertical_Device/2021/v1/RP_Vertical_Device.xml
to
Geometry/VeryForwardData/data/RP_Vertical_Device/2021/Simu/v1/RP_Vertical_Device.xml

but there are other codes still requiring
Geometry/VeryForwardData/data/RP_Vertical_Device/2021/v1/RP_Vertical_Device.xml
so should we keep it? or should we touch those other codes?

We should keep both even if they are the same file. We moved it but we should have copied it. We are adding Run3 geometry, reordering Geometry/VeryForwardData/data and keeping a full back compatibility. This PR is an intermediate step.

qliphy · 2021-07-22T09:20:03Z

Thanks @fabferro I saw you now add the missing file in cms-data/Geometry-VeryForwardData#1

However, as suggested by @smuzaffar #34499 (comment)
it would be better to move all xml files under cmssw/Geometry/VeryForwardData/data to https://github.com/cms-data/Geometry-VeryForwardData
Would you please do that?

fabferro · 2021-07-22T09:23:19Z

Thanks @fabferro I saw you now add the missing file in cms-data/Geometry-VeryForwardData#1

However, as suggested by @smuzaffar #34499 (comment)
it would be better to move all xml files under cmssw/Geometry/VeryForwardData/data to https://github.com/cms-data/Geometry-VeryForwardData
Would you please do that?

I can copy them but this sounds new to me. Are you going to ask me to remove the files from CMSSW?
@cvuosalo can you comment on this request?

qliphy · 2021-07-22T09:29:59Z

@fabferro

You can first make a cms-data PR to copy all xml files under cmssw/Geometry/VeryForwardData/data to https://github.com/cms-data/Geometry-VeryForwardData
then you can make a cms-sw PR to remove all those xml files
We will take care of afterwards steps

On the other hand, if there is no objection, we can also first merge cms-data/Geometry-VeryForwardData#1 to add back the missing file, in order to cure the IB issues. Then you can take actions (above (1) and (2) ) later.

fabferro · 2021-07-22T10:47:41Z

@fabferro
1. You can first make a cms-data PR to copy all xml files under cmssw/Geometry/VeryForwardData/data to https://github.com/cms-data/Geometry-VeryForwardData

2. then you can make a cms-sw PR to remove all those xml files

3. We will take care of afterwards steps
On the other hand, if there is no objection, we can also first merge cms-data/Geometry-VeryForwardData#1 to add back the missing file, in order to cure the IB issues. Then you can take actions (above (1) and (2) ) later.

Let's see if with the PR to cms-data we have cured the IB issues.
Before proceeding to move all PPS xml files to cms-data I would like to understand it better.
1- Are all detectors moving the files to cms-data? I still see tons of them in CMSSW.
2- Will I be able to link them directly in .py or in .xml files inside CMSSW?
As I said it's the first time I hear such a request.
Thanks.

qliphy · 2021-07-22T12:07:26Z

Before proceeding to move all PPS xml files to cms-data I would like to understand it better.
1- Are all detectors moving the files to cms-data? I still see tons of them in CMSSW.

It is now recommended to move files to cms-data.

2- Will I be able to link them directly in .py or in .xml files inside CMSSW?

Yes, it works in many other cases.

cvuosalo · 2021-07-22T13:23:05Z

I must have missed an important announcement. When has it been discussed to move Geometry XML files out of CMSSW?

cvuosalo · 2021-07-22T13:29:11Z

@fabferro I think Geometry/VeryForwardData/data/RP_Vertical_Device/2021/v1/RP_Vertical_Device.xml should be restored in CMSSW.

qliphy · 2021-07-22T13:30:56Z

I must have missed an important announcement. When has it been discussed to move Geometry XML files out of CMSSW?

@smuzaffar or @silviodonato may comment more on this. But I notice this is now more recommended at least for new data files.

As we receive complains about missing file holding Run3 test, @fabferro would you please make a PR to cms-sw to add the missing file? We can then discuss about the migration of geometry XML files to cms-data or not at e.g. next ORP.

fabferro · 2021-07-22T13:31:45Z

I must have missed an important announcement. When has it been discussed to move Geometry XML files out of CMSSW?

@smuzaffar or @silviodonato may comment more on this. But I notice this is now more recommended at least for new data files.

As we receive complains about missing file holding Run3 test, @fabferro would you please make a PR to cms-sw to add the missing file? We can then discuss about the migration of geometry XML files to cms-data or not at e.g. next ORP.

Sure. Doing it.

fabferro · 2021-07-22T13:50:05Z

Done in PR#34588. Code checks ongoing.

fabferro added 3 commits July 15, 2021 12:21

new run3 PPS geometry

595ae58

new ctpp timing file

3e6e008

removing unused dir

c293caf

cmsbuild added this to the CMSSW_12_0_X milestone Jul 15, 2021

cmsbuild added code-checks-pending db-pending geometry-pending orp-pending pending-signatures tests-pending upgrade-pending labels Jul 15, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 15, 2021

cmsbuild added tests-started and removed tests-pending labels Jul 15, 2021

cmsbuild added tests-rejected and removed tests-started labels Jul 15, 2021

cvuosalo reviewed Jul 15, 2021

View reviewed changes

bug fix

3128c6a

fabferro changed the title ~~Geo proposal120pre4~~ PPS geometry for Run3 Jul 15, 2021

adding forgotten files

7c5f15f

cmsbuild removed code-checks-approved tests-rejected labels Jul 15, 2021

cmsbuild added the upgrade-approved label Jul 21, 2021

cmsbuild added orp-approved and removed orp-pending labels Jul 21, 2021

cmsbuild merged commit 2cd5108 into cms-sw:master Jul 21, 2021

This was referenced Jul 21, 2021

scram-tools repo to provide scram toolfile definitions cms-sw/cmsdist#7147

Merged

support global::OutputModule<ExternalWork> #34579

Merged

Remove EventSetup use from PdtEntry #34581

Merged

qliphy mentioned this pull request Jul 22, 2021

Adding missing file cms-data/Geometry-VeryForwardData#1

Open

smuzaffar mentioned this pull request Jul 22, 2021

PPS geometry bug fix: missing file #34588

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PPS geometry for Run3 #34499

PPS geometry for Run3 #34499

fabferro commented Jul 15, 2021

cmsbuild commented Jul 15, 2021

cmsbuild commented Jul 15, 2021

fabferro commented Jul 15, 2021

civanch commented Jul 15, 2021

cmsbuild commented Jul 15, 2021

cvuosalo commented Jul 15, 2021

cvuosalo Jul 15, 2021

fabferro Jul 15, 2021

cvuosalo commented Jul 15, 2021

cmsbuild commented Jul 21, 2021

qliphy commented Jul 21, 2021

qliphy commented Jul 22, 2021

fabferro commented Jul 22, 2021

smuzaffar commented Jul 22, 2021

qliphy commented Jul 22, 2021 •

edited

Loading

fabferro commented Jul 22, 2021 •

edited

Loading

qliphy commented Jul 22, 2021

fabferro commented Jul 22, 2021

qliphy commented Jul 22, 2021

fabferro commented Jul 22, 2021

qliphy commented Jul 22, 2021

fabferro commented Jul 22, 2021

qliphy commented Jul 22, 2021 •

edited

Loading

fabferro commented Jul 22, 2021

qliphy commented Jul 22, 2021

cvuosalo commented Jul 22, 2021

cvuosalo commented Jul 22, 2021

qliphy commented Jul 22, 2021

fabferro commented Jul 22, 2021

fabferro commented Jul 22, 2021

PPS geometry for Run3 #34499

PPS geometry for Run3 #34499

Conversation

fabferro commented Jul 15, 2021

PR description:

PR validation:

cmsbuild commented Jul 15, 2021

cmsbuild commented Jul 15, 2021

fabferro commented Jul 15, 2021

civanch commented Jul 15, 2021

cmsbuild commented Jul 15, 2021

Unit Tests

Comparison Summary

cvuosalo commented Jul 15, 2021

cvuosalo Jul 15, 2021

Choose a reason for hiding this comment

fabferro Jul 15, 2021

Choose a reason for hiding this comment

cvuosalo commented Jul 15, 2021

cmsbuild commented Jul 21, 2021

qliphy commented Jul 21, 2021

qliphy commented Jul 22, 2021

fabferro commented Jul 22, 2021

smuzaffar commented Jul 22, 2021

qliphy commented Jul 22, 2021 • edited Loading

fabferro commented Jul 22, 2021 • edited Loading

qliphy commented Jul 22, 2021

fabferro commented Jul 22, 2021

qliphy commented Jul 22, 2021

fabferro commented Jul 22, 2021

qliphy commented Jul 22, 2021

fabferro commented Jul 22, 2021

qliphy commented Jul 22, 2021 • edited Loading

fabferro commented Jul 22, 2021

qliphy commented Jul 22, 2021

cvuosalo commented Jul 22, 2021

cvuosalo commented Jul 22, 2021

qliphy commented Jul 22, 2021

fabferro commented Jul 22, 2021

fabferro commented Jul 22, 2021

qliphy commented Jul 22, 2021 •

edited

Loading

fabferro commented Jul 22, 2021 •

edited

Loading

qliphy commented Jul 22, 2021 •

edited

Loading