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

Updating phase2 Tracker Geometry (T3) - 2023D4 scenario #16896

Merged
merged 9 commits into from Dec 9, 2016

Conversation

boudoul
Copy link
Contributor

@boudoul boudoul commented Dec 7, 2016

Greetings - This PR is putting in place the xmls prepared by @ghugo83 -
This is the version (internal tracker version : OT365+ Pixel4026) of the Tracker from Tklayout superseeding the previous version (OT362 + Pixel4021) - updating materials and inner + outer modules position

This was presented here : https://indico.cern.ch/event/536862/contributions/2390748/attachments/1382904/2103143/mersi_20161205_newlayouts.pdf with the corresponding mat budget plots

Affecting only 2023D4 scenario

At the same time I updated the recomaterial analyzer to deal with the new material (and cleaned obsolete customize function in them )

adding usual watchers : @delaere , @ atricomi , @rovere , @VinInn @makortel @ebrondol @venturia

Tested with WF# 21210

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 7, 2016

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

It involves the following packages:

Configuration/Geometry
Geometry/CMSCommonData
Geometry/TrackerCommonData
Geometry/TrackerRecoData
Geometry/TrackerSimData
SimTracker/TrackerMaterialAnalysis

@civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @makortel, @sdevissc, @GiacomoSguazzoni, @rovere, @VinInn, @Martin-Grunewald, @ghugo83, @threus, @dgulhan, @venturia this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@boudoul
Copy link
Contributor Author

boudoul commented Dec 7, 2016

Adding for real @atricomi [typo in the description above ]

@boudoul
Copy link
Contributor Author

boudoul commented Dec 7, 2016

Adding @kpedro88

@ghugo83
Copy link
Contributor

ghugo83 commented Dec 7, 2016

@boudoul Ah just a very very minor thing :
In : SimTracker/TrackerMaterialAnalysis/test/listIds_PhaseII.py
pixel:tkLayout_SenSi doesn't exist,
materials = cms.untracked.vstring("materials:Silicon", "tracker:tkLayout_SenSi"),
would be enough

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 7, 2016

Pull request #16896 was updated. @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @davidlange6 can you please check and sign again.

@boudoul
Copy link
Contributor Author

boudoul commented Dec 7, 2016

@ghugo83 , thanks, good catch, just fixed this in my last commit

@kpedro88
Copy link
Contributor

kpedro88 commented Dec 7, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 7, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/16833/console Started: 2016/12/07 16:06

@civanch
Copy link
Contributor

civanch commented Dec 7, 2016

@boudoul , in pixelProdCuts.xml in line 18

10 cm better to substitute to 1 mm.

@ghugo83
Copy link
Contributor

ghugo83 commented Dec 7, 2016

Thanks @civanch :) In trackerProdCuts.xml you mean ?
And what about the other values in pixel/tracker ProdCuts.xml, are they ok ?
Once this is checked one can also remove the "warning" line in the XML :)

@civanch
Copy link
Contributor

civanch commented Dec 7, 2016

@boudoul , other values are fine to me. I have checked trackerProdCuts.xml and pixelProdCuts.xml

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 7, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 7, 2016

Comparison job queued.

@ghugo83
Copy link
Contributor

ghugo83 commented Dec 8, 2016

Just as a comment, following fixes / cross-checks have been done :

  • 0 overlap within Tracker volume, both with Fireworks and Geant4 tools.
  • Sim Material Budget as expected with respect to tkLayout (will provide MB plots after i am finished with the flat tracker baby)

@kpedro88
Copy link
Contributor

kpedro88 commented Dec 8, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 8, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/16863/console Started: 2016/12/08 18:35

@slava77
Copy link
Contributor

slava77 commented Dec 8, 2016 via email

@kpedro88
Copy link
Contributor

kpedro88 commented Dec 8, 2016

@slava77 that's a good idea. I need to think about exactly how it should work. Are there any similar examples elsewhere in CMSSW?

@slava77
Copy link
Contributor

slava77 commented Dec 8, 2016 via email

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 8, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 8, 2016

Comparison job queued.

@ebrondol
Copy link
Contributor

ebrondol commented Dec 8, 2016

I have checked the eff/fake performance of this PR on the global tracking reconstruction. The test have been done with ttbar tkOnly workflows with the IB CMSSW_9_0_X_2016-12-07-1100 @ the commit 78a9963. No PU included.
The results are consistent with "small changes" in the reconstruction. Here reported some important plots:
16896_eff
16896_hits

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 8, 2016

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

The workflows 1003.0, 1001.0, 1000.0, 140.53, 136.731, 4.22 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

@ianna
Copy link
Contributor

ianna commented Dec 8, 2016

+1

@civanch
Copy link
Contributor

civanch commented Dec 8, 2016

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 8, 2016

This pull request is fully signed and it will be integrated in one of the next CMSSW_9_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 382c220 into cms-sw:CMSSW_9_0_X Dec 9, 2016
@ghugo83
Copy link
Contributor

ghugo83 commented Dec 12, 2016

As promised, comparison of the CMSSW Sim Material Budget with tkLayout MB (was done quickly on the fly after creating the XMLs, proper plots now :) )
This is the MB within Full Tracker volume (Beam Pipe excluded). 10000 tracks.
4026_comparison

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

9 participants