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

Update material budget for Phase 2 Flat and Tilted Trackers #15946

Merged
merged 3 commits into from Sep 23, 2016

Conversation

ghugo83
Copy link
Contributor

@ghugo83 ghugo83 commented Sep 22, 2016

Updated Flat and Tilted Tracker material budget. This should (hopefully) be a step towards solving the issues in Phase 2 Reco spotted by @ebrondol .

In this PR :

  • I have updated paths in trackerRecoMaterial.xml and trackerProdCuts.xml to be in concordance with the Pixel / Outer Tracker volumes split.
  • TrackerXi values in TrackerRecoMaterial.xml have been manually hacked by Stefano (thanks @VinInn @rovere @venturia ).

Should I include :

  • fixed a bug (or trick ?) which had been spotted for years in the elementary materials description, and for some reason not fixed. I do not know how these values are used though, but I would say having an accurate elementary materials description cannot harm ... ?

@boudoul @ianna

Question : There is a similar situation in SLHCDEV. Should I do a PR there as well ?

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

Geometry/TrackerRecoData
Geometry/TrackerSimData

@civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @davidlange6 can you please review it and eventually sign? Thanks.
@makortel, @ghellwig, @VinInn this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@VinInn
Copy link
Contributor

VinInn commented Sep 22, 2016

@cmsbuild , please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 22, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/15325/console

@VinInn
Copy link
Contributor

VinInn commented Sep 22, 2016

unable to load /cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc530/cms/cmssw-patch/CMSSW_8_1_X_2016-09-21-2300/lib/slc7_amd64_gcc530/poisoned/plugin-poisoned-IOMCParticleGuns.so because /cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc530/cms/cmssw-patch/CMSSW_8_1_X_2016-09-21-2300/lib/slc7_amd64_gcc530/poisoned/plugin-poisoned-IOMCParticleGuns.so: cannot open shared object file: No such file or directory

will try with an older release...

@VinInn
Copy link
Contributor

VinInn commented Sep 22, 2016

those at CERN can have a look of the DQM for 500 20406.0_SingleMuPt1+SingleMuPt1_2023D2_GenSimFull+DigiFull_2023D2+RecoFullGlobal_2023D2+HARVESTFullGlobal_2023D2
in

http://mrovere-dqmgui-slc6.cern.ch:8060/dqm/dev/start?runnr=1;dataset=/RelValSingleMu1GeVUPFlat/CMSSW_8_1_0/RECO;sampletype=offline_relval;filter=all;referencepos=overlay;referenceshow=customise;referencenorm=True;referenceobj1=refobj;referenceobj2=none;referenceobj3=none;referenceobj4=none;search=;striptype=object;stripruns=;stripaxis=run;stripomit=none;workspace=Everything;size=M;root=Tracking/TrackParameters/generalTracks/GeneralProperties;focus=;zoom=no;

bottom line "is ok"
please integrate this PR as soon as possible and make such that official samples are produced during the weekend.
this is critical to have CMS presenting tracking results at the ECFA workshop

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

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

@VinInn
Copy link
Contributor

VinInn commented Sep 22, 2016

@cmsbuild , please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 22, 2016

@ghugo83
Copy link
Contributor Author

ghugo83 commented Sep 22, 2016

Thanks to Stefano (@alkemyst) advice, I am reintroducing the elementary material fixes.
Thanks a lot to Stefano for this, he was the one to provide new physical parameters in Z and A calculation formulae, I have just been copying it into the code.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

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

@VinInn
Copy link
Contributor

VinInn commented Sep 22, 2016

mtv for 300 ttbar
20024.1_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2023D1_GenSimFull+DigiFull_2023D1+RecoFull_trackingOnly_2023D1+HARVESTFull_trackingOnly_2023D1
http://innocent.home.cern.ch/innocent/RelVal/TTBarUPTilted/

@cmsbuild
Copy link
Contributor

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

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

  • 10424.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017NewFPix_GenSimFull+DigiFull_2017NewFPix+RecoFull_2017NewFPix+HARVESTFull_2017NewFPix

@slava77
Copy link
Contributor

slava77 commented Sep 22, 2016

On 9/22/16 8:06 AM, Vincenzo Innocente wrote:

mtv for 300 ttbar
20024.1_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2023D1_GenSimFull+DigiFull_2023D1+RecoFull_trackingOnly_2023D1+HARVESTFull_trackingOnly_2023D1
http://innocent.home.cern.ch/innocent/RelVal/TTBarUPTilted/

Is there a baseline available to add to this for comparisons?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#15946 (comment), or
mute the thread
https://github.com/notifications/unsubscribe-auth/AEdcbqu-wEjrx-nUUD9Ce1bL-g_l0pcnks5qspldgaJpZM4KDwT_.

@VinInn
Copy link
Contributor

VinInn commented Sep 22, 2016

@venturia
Copy link
Contributor

@ghugo83 I would fix also SLHCDEV if it is not too complicated.

@davidlange6
Copy link
Contributor

apologies, I'm slow - where do these atomic weights come from? (its indeed an improvement that we get the right atomic number for basic elements...)

@cmsbuild
Copy link
Contributor

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

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

  • 10424.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017NewFPix_GenSimFull+DigiFull_2017NewFPix+RecoFull_2017NewFPix+HARVESTFull_2017NewFPix

@slava77
Copy link
Contributor

slava77 commented Sep 22, 2016

On 9/22/16 9:02 AM, Vincenzo Innocente wrote:

http://ebrondol.web.cern.ch/ebrondol/Migration81X/MTV_TTbar_810pre11vsSLHC22/plots_ootb/
???
the reference is SLHC28 (aka TP)

the efficiency drops by up to 5% around 1 GeV

the fake rate goes up by a factor of a few in the eta~2-3 region.

Is this purely an effect of the material budget/geometry updates here?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#15946 (comment), or
mute the thread
https://github.com/notifications/unsubscribe-auth/AEdcbrhUInhRgMt28e3rdES4nUpHIvZvks5qsqamgaJpZM4KDwT_.

@VinInn
Copy link
Contributor

VinInn commented Sep 22, 2016

No clue. to be studies further. The pixel is still PhaseI and for sure sim and reco geometry do not match 100% (understatement).
In any case it is better that what presently in 810, and we need it now

@ghugo83
Copy link
Contributor Author

ghugo83 commented Sep 22, 2016

@venturia Ok, I just did the twin sister PR to SLHCDEV (#15963)

@davidlange6
Copy link
Contributor

maybe there is a more constructive answer to my question from @ghugo83 ? While its good we now know the correct atomic number of Aluminum, its likely not a good strategy to blindly go forward.

@ghugo83
Copy link
Contributor Author

ghugo83 commented Sep 23, 2016

@davidlange6 Sorry I did not answer earlier, was in a plane and at a wedding right now :/
This corresponds to formulae established and tested by Stefano (@alkemyst, stefano.mersi@cern.ch). I let you see with him for any issue.
Z and A are calculated from radiation and interaction lengths.
I am gonna contact him as well from my side.
Cheers

@davidlange6
Copy link
Contributor

ok thanks - I guess we are just lucky that no one else has defined ElementaryMaterials with these names. If these are really derived and not physical, lets get them changed to less generic names to avoid future problems (in a pR to come soon..)

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 1e5595f into cms-sw:CMSSW_8_1_X Sep 23, 2016
@alkemyst
Copy link

Hi @davidlange6
Short answer: these materials are good to go for us. These are computed within tkLayout to obtain the corrrect interaction and radiation lengths for the materials we actually use. A proper fix will be done as next big development in tkLayout. It has been on our TODO list for a long time, but never the most urgent.

A slightly linger answer in tkLayout we use mainly Copper, Aluminium and then a lot of mixtures. We do not have any way (for now) to propagate mixtures inside tkLayout, so we came up with the idea of generating elementary materials for elements like 'Glass', which (as every high school student knows) is an atom with Z=14 and A=28.2688. Since Z must be an integer number, unfortunately we cannot perfectly reproduce rad_length adn interaction_length, so the computation is made in a way that the error on these two is equal and limited to <4%.
As a result A is slightly off for atomic materials (for what it matters) and quite accurate of Cu and Al, which are by and large the only elementary materials we actually use.
Please remember that our knowledge of the material composition of this detector is not more accurate than a few percent anyway, so this approximation is not affecting the results.

Referring to your last comment: in the next iteration of the material exports we will
a) refer to these materials as tkLayout_Cu and so on
b) avoid multiple definitions of the same composition
but here I'm getting OT.

Hope this helps clarifying. For co
error_a
mpleteness I attach a plot showing the error on A we obtain with this method.

Stefano

P.S.
+1

@civanch
Copy link
Contributor

civanch commented Sep 23, 2016

Hi all,
in recent Geant4, when an element is created with given Z a natural isotope composition is retrieved from G4 DB. However, having correct A is indeed useful because in a few computations an average A is still used. You may be use average A in RECO.

@ghugo83
Copy link
Contributor Author

ghugo83 commented Sep 26, 2016

@davidlange6 Ok, to sum-up, in a next PR :

  • Materials will be renamed to tkLayout_Cu and so on.
  • The geometry definition in these XMLs is nearly as short as possible. The materials definition is clearly not as short as possible, I will have a look at this, as it looks like can be very easily reduced.
  • @slava77 TOB and TID can also be renamed in trackerRecoMaterial.xml if you want.

Off-topic note : The situation on the materials is very similar in Tilted4021 in 81X. I have prepared the equivalent changes. @boudoul : PR ?

Cheers

@slava77
Copy link
Contributor

slava77 commented Sep 26, 2016

On 9/26/16 10:34 AM, ghugo83 wrote:

@davidlange6 https://github.com/davidlange6 Ok, to sum-up, in a next PR :

  • Materials will be renamed to tkLayout_Cu and so on.
  • The geometry definition in these XMLs is nearly as short as
    possible. The materials definition is clearly not as short as
    possible, I will have a look at this, as it looks like can be very
    easily reduced.
  • @slava77 https://github.com/slava77 TOB and TID can also be
    renamed in trackerRecoMaterial.xml if you want.

It would be nice.
Thanks.

Off-topic note : The situation on the materials is very similar in
Tilted4021 in 81X. I have prepared the equivalent changes. @boudoul
https://github.com/boudoul : PR ?

Cheers


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#15946 (comment), or
mute the thread
https://github.com/notifications/unsubscribe-auth/AEdcbhUdWaRgHtlZnmf_rd2nIk_jLU9xks5quAIYgaJpZM4KDwT_.

@ghugo83
Copy link
Contributor Author

ghugo83 commented Sep 28, 2016

@davidlange6 NB : The elementary materials Z and A are at the status of what they were before this PR in many releases. This traces back from the time of LongBarrel6PS or BarrelEndcap5D in CMSSW_6_1_X_SLHC for example.
https://github.com/cms-sw/cmssw/tree/CMSSW_6_1_X_SLHC/Geometry/TrackerCommonData/data/PhaseII

@davidlange6
Copy link
Contributor

sure - but given the danger of conflicts (and worse if G4 is changing the definitions) - we should change going forward (as I believe is agreed)

On Sep 28, 2016, at 11:05 AM, ghugo83 notifications@github.com wrote:

@davidlange6 NB : The elementary materials Z and A are at the status of what they were before this PR in many releases. This traces back from the time of LongBarrel6PS or BarrelEndcap5D in CMSSW_6_1_X_SLHC for example.
https://github.com/cms-sw/cmssw/tree/CMSSW_6_1_X_SLHC/Geometry/TrackerCommonData/data/PhaseII


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@ghugo83
Copy link
Contributor Author

ghugo83 commented Oct 27, 2016

This PR is for :

  • Geometry/TrackerCommonData/data/PhaseII/FlatTracker/ in 81X
  • Geometry/TrackerCommonData/data/PhaseII/TiltedTracker/ in 81X

But not :

  • Geometry/TrackerCommonData/data/PhaseII/FlatTracker/ in SLHCDEV
  • Geometry/TrackerCommonData/data/PhaseII/TiltedTracker/ in SLHCDEV
  • Geometry/TrackerCommonData/data/PhaseII/TiltedTracker4021/ in 81X

Please see : #15946 for SLHCDEV.
#16373 for TiltedTracker4021 in 81X

@VinInn
Copy link
Contributor

VinInn commented Oct 27, 2016

type urgent

@boudoul
Copy link
Contributor

boudoul commented Oct 27, 2016

the urgenit is not for this one which is already merged for a month but for #16373

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

10 participants