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

Phase2 Outer Tracker : Incorporating recent changes from TEDD Design + cleaning #22873

Merged
merged 4 commits into from Apr 18, 2018

Conversation

boudoul
Copy link
Contributor

@boudoul boudoul commented Apr 6, 2018

Greetings, this PR aims to introduce in the dev Phase2 Tracker (=T6,T7,T8) the changes from Mechanical design studies in the TEDD - The changes were presented here https://indico.cern.ch/event/709594/contributions/2931560/attachments/1615255/2566756/upgSim180312_GB.pdf - Those are so small that basically no regression should be observed.
The changes in the geometry were done by @ghugo83 .

I also took the opportunity clean previous detids files which were either not used, or not well named , so that now they are associated to the corresponding Tracker (T5 , T6 , T7, T8, ..) appropriately instead of previous cryptic names ; and therefore will be much help some future developments in progress related to cabling map .
@emiglior FYI

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2018

The code-checks are being triggered in jenkins.

@boudoul
Copy link
Contributor Author

boudoul commented Apr 6, 2018

this PR should be tested with the files from the externals that I just created : cms-data/SLHCUpgradeSimulations-Geometry#10

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2018

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2018

A new Pull Request was created by @boudoul (boudoul) for master.

It involves the following packages:

Configuration/Geometry
Geometry/TrackerCommonData
Geometry/TrackerRecoData
Geometry/TrackerSimData
SLHCUpgradeSimulations/Geometry

@civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @kpedro88 can you please review it and eventually sign? Thanks.
@ghellwig, @makortel, @VinInn, @Martin-Grunewald, @ebrondol, @ghugo83, @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

@mrodozov
Copy link
Contributor

mrodozov commented Apr 6, 2018

please test with cms-sw/cmsdist#3901

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2018

The tests are being triggered in jenkins.
Using externals from cms-sw/cmsdist#3901
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/27341/console

@boudoul
Copy link
Contributor Author

boudoul commented Apr 6, 2018

thank you @mrodozov for the super prompt reaction !

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2018

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2018

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2018

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 2646 differences found in the comparisons
  • DQMHistoTests: Total files compared: 29
  • DQMHistoTests: Total histograms compared: 2504254
  • DQMHistoTests: Total failures: 4078
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2500000
  • DQMHistoTests: Total skipped: 176
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.8400000002 KiB( 21 files compared)
  • Checked 119 log files, 9 edm output root files, 29 DQM output files

@kpedro88
Copy link
Contributor

Changes are isolated to 21234.0 (D21 detector w/ T6), appears to be caused by statistical fluctuations from differences in the simulation history (expected for any geometry change)

@kpedro88
Copy link
Contributor

+1

@ianna
Copy link
Contributor

ianna commented Apr 13, 2018

+1

@cmsbuild
Copy link
Contributor

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)

@fabiocos
Copy link
Contributor

@boudoul @ghugo83 @kpedro88 I see that this PR, that for the rest looks ok to enter into the release, is changing the tracker scenario V404, about which we were discussing for the MTD addition. From the initial comment of Gaelle I understand that the expected changes are so small to be negligible, but they are non zero. So up to which point do we consider this scenario as frozen?

@emiglior
Copy link
Contributor

@boudoul @ghugo83 @fabiocos @kpedro88
My understanding is that there are no short term changes foreseen for the OuterTracker with respect those in this PR (OT v.6.1.4) but we expect changes for the InnerTracker compared to IT v4.0.4

@fabiocos
Copy link
Contributor

@emiglior thank you Ernesto, that is ok, but I assume it will be a new version. here we are discussing about changes, although minor, in the very same version, and a similar discussion is ongoign about the possibility to refactor the geometry structure in these existing scenarios to accomodate the new BTL volume and have a separate OTST volume. For me, provided that the geometry and material distributions are unchanged it is not a big issue if there is a valid reason (and we validate that there is no change)

@fabiocos
Copy link
Contributor

In any case, I understand that this D21 scenario has been used so far in tests and not in massive productions, am I correct? If this is the case, I will integrate the package this evening

@emiglior
Copy link
Contributor

In any case, I understand that this D21 scenario has been used so far in tests and not in massive productions, am I correct?

I would say so. 2023D17 is the TDR geometry used for large productions; 2023D21 is the devel geometry used so far only for RelVal (SingleMu, TTbar, MinBias) in 10_0_0_pre1 (@boudoul may confirm)

@boudoul
Copy link
Contributor Author

boudoul commented Apr 18, 2018

@fabiocos I confirm, the 'frozen' scenario is D17 , it does contain the TDR version and has been (is) the subject of MC production - the tracker within D21 is considered as a development one and is not for MC prod (at this point)

@fabiocos
Copy link
Contributor

cms-sw/cmsdist/#3901 merged, merge also this package

@fabiocos
Copy link
Contributor

+1

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

7 participants