PPS: update of Validation/CTPPS #28492

jan-kaspar · 2019-11-27T15:58:50Z

PR description:

This is an update of code in Validation/CTPPS which has been used extensively in 2019 to prepare proton-reconstruction SW and conditions for the UL re-reco. It also contains "direct" simulation used to study proton-reconstruction uncertainties and more.

It was necessary to make several changes outside the Validation/CTPPS directory. These changes are mostly small and technical (details follow) and introduce no change in proton-recontsruction results (see below).

CalibPPS/ESProducers

new auxiliary modules added
for several modules: configurable input/output label

CondFormats/CTPPSReadoutObjects

a friend declaration added

CondFormats/DataRecord

CTPPSBeamParametersRcd made dependent on LHCInfoRcd (allows to change parameters with xangle)

DataFormats/CTPPSReco

few more dictionaries generated

Geometry/VeryForwardGeometryBuilder

store sensor type (to distinguish between pixel 2x2 and 3x2 modules)

RecoCTPPS/ProtonReconstruction

configurable input-optics label
timing-RP local track also stored in the reco proton contributor list

PR validation:

runTheMatrix.py -l limited --ibeos resulted in 30 29 28 22 16 4 1 1 1 tests passed, 4 0 0 0 0 0 0 0 0 failed. The four failing tests (20034.0, 20434.0, 21234.0, 22034.0) reported:

Exception: 'Phase2C4_timing_layer_bar' is not a valid option for '--era

thus most likely unrelated to this PR.

Below a comparison of proton reconstruction performed with CMSSW_11_0_0_pre12 (blue) and this PR (red dashed) - there is no difference.

cmsbuild · 2019-11-27T15:59:22Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-11-27T16:05:22Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28492/12937

This PR adds an extra 76KB to repository

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

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

cmsbuild · 2019-11-28T07:57:55Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-11-28T08:04:22Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28492/12943

This PR adds an extra 76KB to repository

cmsbuild · 2019-11-28T08:04:52Z

A new Pull Request was created by @jan-kaspar for master.

It involves the following packages:

CalibPPS/ESProducers
CondFormats/CTPPSReadoutObjects
CondFormats/DataRecord
DataFormats/CTPPSReco
Geometry/VeryForwardGeometryBuilder
RecoCTPPS/ProtonReconstruction
Validation/CTPPS

@perrotta, @andrius-k, @Dr15Jones, @schneiml, @kmaeshima, @cvuosalo, @civanch, @tlampen, @christopheralanwest, @ianna, @mdhildreth, @cmsbuild, @franzoni, @jfernan2, @fioriNTU, @slava77, @ggovi, @pohsun, @tocheng can you please review it and eventually sign? Thanks.
@forthommel, @rovere, @mmusich, @seemasharmafnal, @tocheng this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

jfernan2 · 2019-11-28T08:22:57Z

please test

cmsbuild · 2019-11-28T08:23:28Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/3677/console Started: 2019/11/28 09:31

pohsun · 2019-12-03T08:30:52Z

+1

cvuosalo · 2019-12-03T20:10:03Z

Geometry/VeryForwardGeometryBuilder/src/DetGeomDesc.cc

+  if (found != std::string::npos && sensor_name.substr(found - 4, 3) == DDD_CTPPS_PIXELS_SENSOR_TYPE_2x2) {
+    m_sensorType = DDD_CTPPS_PIXELS_SENSOR_TYPE_2x2;
+  }
+}


It would be good to create a second version of this constructor that takes the new DD4hep cms::DDFilteredView, either in this PR or a follow-up PR. We are at the end of the DD4hep migration, so this code that uses old DD will need to be eliminated in a few months.

Absolutely correct! The DD4Hep migration (and reco-geometry storage in DB) have been started in parallel. @malbouis is leading this task. The ETA is March 2020 - is this OK? Our roadmap is sketched here: https://indico.cern.ch/event/866706/contributions/3652254/attachments/1952609/3242097/pps_geometry_dd4hep.pdf .

@cvuosalo , I think, that migration to DD4hep should be done in following PRs not here.

@jan-kaspar Thanks for the info. That is very helpful.

cvuosalo · 2019-12-04T18:54:16Z

+1

jan-kaspar · 2019-12-05T08:52:27Z

@ggovi May I please ask for review? Db is the last signature missing and the changes in CondFormats are small and technical:

LHCInterpolatedOpticalFunctionsSet.h: only a friend declaration added
CTPPSBeamParametersRcd.h: made dependent on LHCInfoRcd (to allow updates upon crossing-angle changes)

Thanks in advance!

ggovi · 2019-12-05T09:58:52Z

+1

cmsbuild · 2019-12-05T09:59:27Z

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

jan-kaspar · 2019-12-05T10:06:34Z

Thanks @ggovi !

fabiocos

@jan-kaspar why are alignment configuration files store inside Validation/CTPPS instead of some alignment package?

fabiocos · 2019-12-09T10:22:03Z

CalibPPS/ESProducers/plugins/CTPPSLHCInfoRandomXangleESSource.cc

+edm::ESProducts<std::unique_ptr<LHCInfo>> CTPPSLHCInfoRandomXangleESSource::produce(const LHCInfoRcd &) {
+  auto output = std::make_unique<LHCInfo>();
+
+  // TODO: this doesn't work, use temporarily ROOT code


@jan-kaspar @pohsun why all this? There is no reproducibility guaranteed in this code as far as I can see...

Sorry, I should have pre-discussed with you. As you may know, the xangle has an important impact on what/how PPS can see. I've been asked by the POG to make a simulation where the xangle would be generated randomly according to a specified histogram (e.g. as from LHC data for a given JSON selection). My attempt is the code currently in the PR and I acknowledge it has issues. What would be your recommendation?

jan-kaspar · 2019-12-09T10:54:28Z

@jan-kaspar why are alignment configuration files store inside Validation/CTPPS instead of some alignment package?

The "direct simulation" comes with settings for 5 periods (2016 pre-TS2 and post-TS2, 2017 pre-TS2 and post-TS2, 2018) which reflect the most important changes of conditions relevant for PPS in Run2. The settings consist of python configs and alignment files. Currently, the python configs are in Validation/CTPPS/python/simu_config. In order to have the alignment files in a similar place, they were put in Validation/CTPPS/alignment. Would you have a better suggestion?

fabiocos · 2019-12-11T17:28:56Z

@jan-kaspar I understand that this is a kind of non-standard validation package, so for the few comments I made I can accept the proposed solutions. We have other cases of calibration ESProducers using random numbers with various kind of configurations. In principle CLHEP might be used here, and I understand that the lack of reproducibility is known and acceptable for the purpose of this code.

jan-kaspar · 2019-12-11T18:50:09Z

Thanks @fabiocos ! I will push (~tomorrow) a small patch to use CLHEP instead of ROOT and to have a python-configurable seed parameter for at least a basic reproducibility.

fabiocos · 2019-12-11T21:19:04Z

+1

PPS: backport of #28492, #28615 and #28621

jan-kaspar added 3 commits November 26, 2019 18:53

Cumulative commit with updates in Validation/CTPPS.

82a6445

Cumulative commit for updates outside Validation/CTPPS.

afad251

Remaining changes imported.

3f3b4a2

cmsbuild added this to the CMSSW_11_0_X milestone Nov 27, 2019

cmsbuild added alca-pending code-checks-pending comparison-pending db-pending dqm-pending geometry-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Nov 27, 2019

cmsbuild added code-checks-rejected and removed code-checks-pending labels Nov 27, 2019

Fixed code formatting.

37500a9

cmsbuild added code-checks-pending and removed code-checks-rejected labels Nov 28, 2019

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 28, 2019

cmsbuild added tests-started and removed tests-pending labels Nov 28, 2019

cmsbuild added the reconstruction-approved label Dec 2, 2019

cmsbuild added alca-approved and removed alca-pending labels Dec 3, 2019

christopheralanwest mentioned this pull request Dec 3, 2019

Optical functions as used for UL re-reco 2016-2018 cms-data/CalibPPS-ESProducers#2

Merged

cvuosalo reviewed Dec 3, 2019

View reviewed changes

cmsbuild added geometry-approved and removed geometry-pending labels Dec 4, 2019

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

fabiocos reviewed Dec 9, 2019

View reviewed changes

cmsbuild added orp-approved and removed orp-pending labels Dec 11, 2019

cmsbuild merged commit c26f4e8 into cms-sw:master Dec 11, 2019

jan-kaspar mentioned this pull request Dec 12, 2019

PPS: address comments #28615

Merged

jan-kaspar added a commit to CTPPS/cmssw that referenced this pull request Jan 8, 2020

Cumulative backport of cms-sw#28492, cms-sw#28615 and cms-sw#28621.

49f3bac

jan-kaspar mentioned this pull request Jan 8, 2020

PPS: backport of #28492, #28615 and #28621 #28720

Merged

cmsbuild added a commit that referenced this pull request Jan 21, 2020

Merge pull request #28720 from CTPPS/pps_backport_10_6

02f73e2

PPS: backport of #28492, #28615 and #28621

jan-kaspar deleted the pps_validation_update_11_0_0 branch January 24, 2020 17:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PPS: update of Validation/CTPPS #28492

PPS: update of Validation/CTPPS #28492

jan-kaspar commented Nov 27, 2019

cmsbuild commented Nov 27, 2019

cmsbuild commented Nov 27, 2019

cmsbuild commented Nov 28, 2019

cmsbuild commented Nov 28, 2019

cmsbuild commented Nov 28, 2019

jfernan2 commented Nov 28, 2019

cmsbuild commented Nov 28, 2019 •

edited

Loading

pohsun commented Dec 3, 2019

cvuosalo Dec 3, 2019

jan-kaspar Dec 4, 2019

civanch Dec 4, 2019

cvuosalo Dec 4, 2019

cvuosalo commented Dec 4, 2019

jan-kaspar commented Dec 5, 2019

ggovi commented Dec 5, 2019

cmsbuild commented Dec 5, 2019

jan-kaspar commented Dec 5, 2019

fabiocos left a comment

fabiocos Dec 9, 2019

jan-kaspar Dec 9, 2019

jan-kaspar commented Dec 9, 2019

fabiocos commented Dec 11, 2019

jan-kaspar commented Dec 11, 2019

fabiocos commented Dec 11, 2019

PPS: update of Validation/CTPPS #28492

PPS: update of Validation/CTPPS #28492

Conversation

jan-kaspar commented Nov 27, 2019

PR description:

PR validation:

cmsbuild commented Nov 27, 2019

cmsbuild commented Nov 27, 2019

cmsbuild commented Nov 28, 2019

cmsbuild commented Nov 28, 2019

cmsbuild commented Nov 28, 2019

jfernan2 commented Nov 28, 2019

cmsbuild commented Nov 28, 2019 • edited Loading

pohsun commented Dec 3, 2019

cvuosalo Dec 3, 2019

Choose a reason for hiding this comment

jan-kaspar Dec 4, 2019

Choose a reason for hiding this comment

civanch Dec 4, 2019

Choose a reason for hiding this comment

cvuosalo Dec 4, 2019

Choose a reason for hiding this comment

cvuosalo commented Dec 4, 2019

jan-kaspar commented Dec 5, 2019

ggovi commented Dec 5, 2019

cmsbuild commented Dec 5, 2019

jan-kaspar commented Dec 5, 2019

fabiocos left a comment

Choose a reason for hiding this comment

fabiocos Dec 9, 2019

Choose a reason for hiding this comment

jan-kaspar Dec 9, 2019

Choose a reason for hiding this comment

jan-kaspar commented Dec 9, 2019

fabiocos commented Dec 11, 2019

jan-kaspar commented Dec 11, 2019

fabiocos commented Dec 11, 2019

cmsbuild commented Nov 28, 2019 •

edited

Loading