Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Phase-2 InnerTracker conditions from DB (version 2) #27517

Merged
merged 8 commits into from Aug 12, 2019

Conversation

mmusich
Copy link
Contributor

@mmusich mmusich commented Jul 13, 2019

PR description:

Upcoming developments in the Phase-2 Inner Tracker digitization and local reconstruction require access to several conditions related to the CPE, namely per-module Pixel Lorentz Angle (both for simulation and reconstruction) and in future also Pixel 1D, 2D Templates and Generic Errors.
Currently only the generic CPE algorithm is used for the Inner Tracker local reconstruction (hence only the Lorentz Angle is accessed) and it is is provided via a fake conditions ESSource: SiPixelFakeLorentzAngleESSource. This ESSource makes use of the DetId list from a “Skimmed Geometry” external file: one DetId list for each geometry (example) and only one single LA (μH =0.106) for all IT modules is used (hardcoded).
Moreover the “forWidth” label is not taken from DB (see PixelCPEGeneric_cfi configuration) hence no correction is applied, and the simulation LA value is passed as a configuration parameter in the digitizer (see phase2TrackerDigitizer_cfi.).
Finally the Generic Errors are also not taken from DB (see PixelCPEGeneric_cfi) but are hard-coded in C++ (see PixelCPEGeneric.cc).
This PR is meant to change part of this situation, switching on the reading of conditions from Database where payloads are available, and provide such payloads in the production DB.
The implementation is complicated by the co-existence in release of several geometries, with different DetIds lists, and all need to be supported, hence the needed conditions cannot simply be added to the phase2_realistic autoCond Global Tag key to satisfy all use-cases.
In this PR we propose a solution based on the dynamical creation of new autoCond Global Tag keys, one for each of the geometries to be supported, by appending the CPE-related conditions in a similar fashion to what already happens for the HLT GT keys in autoCondHLT.
A complementary approach based on an additional ESSource configured differently for each Tracker T* geometry via the fakeConditions_phase2TkT*_cff.py files (already included in the geometry definition) has been implemented in PR #27516 .
More information has been collected in this presentation at the AlCa/DB meeting of 7th July 2019.
Please notice that a different set of LA values (μH =0.053) is be applied to the T15 geometry with respect to the current value (μH =0.106), hence changes are expected in D45-related workflows.
This PR is RFC and in draft state as it is at odds with the purpose of issue #27393.

PR validation:

The PR was tested running 1 event of SingleMuPt10 for all the currently supported phase-2 geometry combinations: D35 (T6), D41 (T14), D43 (T14), D44 (T14), D45 (T15), D46 (T15) via:

runTheMatrix.py --what upgrade -l 20007.0,20407.0,20807.0,21207.0,21607.0,22007.0 --command='-n 1' -j 4

if this PR is a backport please specify the original PR:

This PR is not a backport
cc:
@tsusa @emiglior @pwittich @christopheralanwest @tlampen @ggovi

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27517/10875

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mmusich (Marco Musich) for master.

It involves the following packages:

Configuration/AlCa
Configuration/PyReleaseValidation
RecoLocalTracker/SiPixelRecHits
SimTracker/SiPhase2Digitizer

@perrotta, @cmsbuild, @prebello, @zhenhu, @slava77, @christopheralanwest, @civanch, @mdhildreth, @pgunnell, @franzoni, @tocheng, @tlampen, @pohsun, @kpedro88 can you please review it and eventually sign? Thanks.
@makortel, @felicepantaleo, @GiacomoSguazzoni, @tocheng, @VinInn, @Martin-Grunewald, @dkotlins, @ebrondol, @rovere, @gpetruc, @mmusich, @threus, @dgulhan 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

Copy link
Contributor

@kpedro88 kpedro88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solution seems more parsimonious and maintainable than version 1.

Ultimately, we may want to move to labeled conditions in a single phase 2 GT, but this seems like a useful intermediate step.

It would be good to integrate this more with the phase 2 geometry/workflow scripts (dict2023Geometry.py, generate2023Geometry.py) in order to automate the relationship of a GT with a geometry scenario and corresponding workflow. For now, this could be done in the same way the suggestion of the Era modifiers is done (a printed message when the geometry is generated, based on a field in the subdetector components dictionary).

Configuration/AlCa/python/autoCondPhase2.py Outdated Show resolved Hide resolved
Configuration/AlCa/python/autoCond.py Show resolved Hide resolved
@slava77
Copy link
Contributor

slava77 commented Jul 22, 2019

@mmusich
please clarify on the status of this PR

@christopheralanwest
Copy link
Contributor

I don't know that these symbolic GTs are supported in official MC production; I've asked PdmV experts for their input. Is it intended that the samples generated using the symbolic GTs are only used in local tests?

@mmusich
Copy link
Contributor Author

mmusich commented Jul 23, 2019

I don't know that these symbolic GTs are supported in official MC production.

These GTs can be supported in official MC production, whatever is run in IB tests can also be run in official relval productions.

Is it intended that the samples generated using the symbolic GTs are only used in local tests?

no. they are meant for the general public.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 9, 2019

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e83448/1942/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 1170 differences found in the comparisons
  • DQMHistoTests: Total files compared: 34
  • DQMHistoTests: Total histograms compared: 2939508
  • DQMHistoTests: Total failures: 1560
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2937607
  • DQMHistoTests: Total skipped: 341
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
  • Checked 142 log files, 14 edm output root files, 34 DQM output files

@fabiocos
Copy link
Contributor

@kpedro88 @slava77 @perrotta @civanch @cvuosalo I'll consider your previous signatures as valid
@christopheralanwest @pohsun I believe this is a cleaner solution, in the spirit of the discussion #27393 please comment or sign

@fabiocos
Copy link
Contributor

@christopheralanwest @pohsun the differences are seen only for the scenario D46 (tracker geometry T15) as in the previously approved tests

@christopheralanwest
Copy link
Contributor

+1

@fabiocos
Copy link
Contributor

+1

@fabiocos
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 36f4410 into cms-sw:master Aug 12, 2019
@cvuosalo
Copy link
Contributor

+1

@slava77
Copy link
Contributor

slava77 commented Aug 12, 2019

@kpedro88
Copy link
Contributor

+upgrade

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet