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

Add Phase 2 TFPX geometries (T17 and T18) #28503

Merged
merged 4 commits into from Dec 4, 2019
Merged

Conversation

ghugo83
Copy link
Contributor

@ghugo83 ghugo83 commented Nov 28, 2019

PR description:

TFPX study.
This PR adds 2 Phase 2 Inner Tracker geometries: T17 and T18, and their associated workflows.
T17 corresponds to most realistic TFPX design, while T18 intends to study the effect of shifting TFPX double-disks in Z.

Tracking performance plots are needed for both designs.
Following comparisons should be made:
T16 versus T17 (2026D48 versus 2026D51).
T17 versus T18 (2026D51 versus 2026D52).

NB: This PR is TFPX-only, does not include recent changes in OT.

PR validation:
Following was done for validation:

  • Solved all overlaps on CMSSW with Fireworks and Geant4 tools.
  • Even if the active geometry is different from T15, logically there should be no DetId diff (based on how the DetIds are computed in CMSSW). Checked that this is the case indeed.
    This allows to reuse the fakeConditions from T15.
  • Checked workflow numbering.
  • Checked that workflows with D51 and D52 run smoothly with no extra error / warning.

FYI: @emiglior @pwittich @bsunanda @fabiocos @kpedro88 @mmusich @jalimena @boudoul

…thin all double-disks + Increased distance between Disks 6 and 7 + Put TBPX portcards between Disks 6 and 7. IT 616, TFPX: Shift all double-disks by + 25mm in Z. All services volumes (& TBPX portcards) are also shifted.
@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@@ -46,5 +48,7 @@ Several detector combinations have been generated:
* D48 = T16+C9+M3+I10+O3+F2
* D49 = T15+C9+M4+I10+O4+F2
* D50 = T15+C9+M4+I11+O4+F2
* D51 = T17+C9+M4+I10+O4+F2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was not clear to me whether I should use M3 or M4, O3 or O4, I10 or I11.
Please let me know in case change is needed here :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you made the right choice to start from D49, since that is the new baseline for the HLT TDR. (M4 and O4 should always go together.)

Copy link
Contributor Author

@ghugo83 ghugo83 Dec 3, 2019

Choose a reason for hiding this comment

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

ok thanks @kpedro88

Copy link
Contributor

Choose a reason for hiding this comment

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

@ghugo83 @kpedro88 O11 is equivalent to O10 as physics content, it is mostly a validation/migration tool for me, D49 is a meaningful base

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28503/12954

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @ghugo83 for master.

It involves the following packages:

Configuration/Geometry
Configuration/PyReleaseValidation
Configuration/StandardSequences
Geometry/CMSCommonData
Geometry/TrackerCommonData

@cmsbuild, @civanch, @Dr15Jones, @chayanit, @cvuosalo, @ianna, @mdhildreth, @pgunnell, @franzoni, @kpedro88, @zhenhu, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@vargasa, @makortel, @felicepantaleo, @GiacomoSguazzoni, @rovere, @VinInn, @Martin-Grunewald, @ebrondol, @dgulhan, @venturia 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

@@ -8,6 +8,8 @@ Tracker:
* T14: Phase2 tilted tracker (v6.1.6) w/ phase 2 pixel (v6.1.3) (Based from T12. OT: reduced envelope. IT: new chip size, different radii, 2x2 modules everywhere in TEPX, new ring paradigm in TEPX)
* T15: Phase2 tilted tracker (v6.1.6) w/ phase 2 pixel (v6.1.3) (Active geometry: same as T14. Material Budget: major update in IT, gathering info from recent Mechanical designs.)
* T16: Active geometry: skewed Inner Tracker geometry. Material Budget: same as T15.
* T17: Phase2 tilted tracker (v6.1.6) w/ phase 2 pixel (v6.1.5) TFPX: Changed sensors spacing within all double-disks + Increased distance between Disks 6 and 7 + Put TBPX portcards between Disks 6 and 7.
* T18: Phase2 tilted tracker (v6.1.6) w/ phase 2 pixel (v6.1.6) TFPX: Shift all double-disks by + 25mm in Z. All services volumes (& TBPX portcards) are also shifted.
Copy link
Contributor

Choose a reason for hiding this comment

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

has this been checked for overlaps?

Copy link
Contributor Author

@ghugo83 ghugo83 Dec 3, 2019

Choose a reason for hiding this comment

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

Yes i always solve all overlaps (it's a show-stopper for tracking) + also did the other checks mentioned in PR validation.

@kpedro88
Copy link
Contributor

kpedro88 commented Dec 2, 2019

please test workflow 23634.0,24034.0

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 2, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/3729/console Started: 2019/12/02 17:59

@kpedro88
Copy link
Contributor

kpedro88 commented Dec 2, 2019

@icosivi @fabiocos please be aware of this PR. Based on our first come first serve policy for detector numbers, the revised version of #28490 should be rebased after this one is merged, moving to number D53 (since D51 is already taken here).

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 2, 2019

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 2, 2019

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 2, 2019

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

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-97f129/23234.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2026D49_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D49+RecoFullGlobal_2026D49+HARVESTFullGlobal_2026D49
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-97f129/23634.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2026D51_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D51+RecoFullGlobal_2026D51+HARVESTFullGlobal_2026D51
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-97f129/24034.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2026D52_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D52+RecoFullGlobal_2026D52+HARVESTFullGlobal_2026D52

Comparison Summary:

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

@kpedro88
Copy link
Contributor

kpedro88 commented Dec 3, 2019

+upgrade

@chayanit
Copy link

chayanit commented Dec 3, 2019

+1

@cvuosalo
Copy link
Contributor

cvuosalo commented Dec 3, 2019

+1

@fabiocos
Copy link
Contributor

fabiocos commented Dec 4, 2019

+operations

the update of StandardSequences is coherent with the purpose of this PR

@fabiocos
Copy link
Contributor

fabiocos commented Dec 4, 2019

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 4, 2019

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 be automatically merged.

@emiglior
Copy link
Contributor

emiglior commented Dec 6, 2019

@fabiocos: is there any chance that D51 and D52 will be part of 11.0 or they will go in 11.1?
(I am interested in requesting one large stat PU200 RelVal sample)

@emiglior
Copy link
Contributor

emiglior commented Dec 6, 2019

"Milestone CMSSW_11_1_X"
thanks @mmusich for pointing it out and sorry for the spam

@fabiocos
Copy link
Contributor

fabiocos commented Dec 6, 2019

@emiglior this is in 11_1_X, as you may also see in https://cmssdt.cern.ch/SDT/html/cmssdt-ib/#/ib/CMSSW_11_1_X . If what you need is a RelVal sample I do not see why this could not be produced in 11_1_0_pre1 (@pgunnell @chayanit please comment in case), of course it depends on what you mean by "large"

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

8 participants