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 I: new BPIX module cable materials, new FPIX portcard material and dimensions #14700

Merged
merged 1 commit into from Jun 1, 2016

Conversation

friccita
Copy link
Contributor

Modifications to Phase I Pixel geometry/material description:

  • BPIX: new module cable materials (one for L1, one for L2-L4)
  • FPIX: new portcard material, new temporary dimensions (to be possibly modified in later iteration of geometry description refinement, after receiving feedback from FNAL engineers)

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @friccita (Francesca Ricci-Tam) for CMSSW_8_1_X.

It involves the following packages:

Geometry/CMSCommonData
Geometry/TrackerCommonData
Validation/Geometry

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

cms-bot commands are list here #13028

@ianna
Copy link
Contributor

ianna commented May 31, 2016

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 31, 2016

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

@ianna
Copy link
Contributor

ianna commented May 31, 2016

+1

@dmitrijus
Copy link
Contributor

+1

@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 after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@makortel
Copy link
Contributor

makortel commented Jun 1, 2016

For my education, could somebody clarify if these changes impact reco geometry and/or alignment, or only passive material is added/changed? Or, in other words, can one run GEN-SIM + DIGI-RECO consistently with this PR alone? I guess using GEN-SIM from 810pre6 or earlier (i.e. essentially runTheMatrix.py -i all - l 10024.0) should continue to work as before with this PR.

@friccita
Copy link
Contributor Author

friccita commented Jun 1, 2016

@makortel These changes don't affect reco geometry or alignment; only passive materials were modified.

@makortel
Copy link
Contributor

makortel commented Jun 1, 2016

Thanks @friccita.

@ghellwig
Copy link

ghellwig commented Jun 1, 2016

@friccita Is the information you are providing here relevant to create MediumProperty objects?

@friccita
Copy link
Contributor Author

friccita commented Jun 1, 2016

@ghellwig I am not familiar with MediumProperty objects; are they derived somehow from the material definitions in xml files?

@ghellwig
Copy link

ghellwig commented Jun 1, 2016

@friccita this is actually, what I wanted to know, too ;)
I recently ran into some problems because this object had no values set for the phase1-FPIX.
See also this HN thread.

@friccita
Copy link
Contributor Author

friccita commented Jun 1, 2016

@ghellwig For me, it's the first time that I've heard of MediumProperty objects... From poking around in the code on Git, I gather that these objects are defined using a radiation length and hadronic interaction length. The material definitions in the Detector Description (DD) do not include any information about radiation lengths or hadronic interaction lengths; the former are calculated by GEANT during simulation using only elemental composition info provided in the xml material definitions. I have defined new materials similarly in the past for the 2015 geometry description, and since MediumProperty issues have not arisen there, then that suggests to me that these new xml material definitions are not related to the creation of MediumProperty objects.

@VinInn
Copy link
Contributor

VinInn commented Jun 1, 2016

@ghellwig
the information provided here IS indeed relevant to create MediumProperty objects.
One has to run a proper "sequence" as outlined by @rovere last Friday

@ghellwig
Copy link

ghellwig commented Jun 1, 2016

@VinInn
I guess you are referring to this talk?
Does that mean, that the proper sequence is not run in the reconstruction of the phase1 relvals?
Or do I have to run the sequence in the user job?

@ianna
Copy link
Contributor

ianna commented Jun 1, 2016

@ghellwig
Yes, it does. In order to include this, the following file needs to be updated, then the changes have to be propagated to a GT:
http://cmslxr.fnal.gov/source/Geometry/TrackerRecoData/data/PhaseI/trackerRecoMaterial.xml?v=CMSSW_8_1_X_2016-05-31-2300
IMHO, it is better to be done centrally rather then in an individual job. Though the latter is possible.

@VinInn
Copy link
Contributor

VinInn commented Jun 1, 2016

@ghellwig , yes that talk
what I mean is that once the Sim geometry is ready one has to generate the proper reco geometry
using the "procedure" (sorry not sequence in terms of workflow) outlined in there

@ghellwig
Copy link

ghellwig commented Jun 1, 2016

@VinInn, @ianna thank you for the clarification and updates!

@mmusich
Copy link
Contributor

mmusich commented Jun 1, 2016

@ianna, @VinInn @rovere can you clarify who's in charge of:
"the following file needs to be updated, then the changes have to be propagated to a GT" ?
Thanks,
M.

@ianna
Copy link
Contributor

ianna commented Jun 1, 2016

@mmusich - Tracker DPG/POG should sign off on the file content.

@VinInn
Copy link
Contributor

VinInn commented Jun 1, 2016

sign off == getting some relval, eventually an alignment excercise...

@mmusich
Copy link
Contributor

mmusich commented Jun 1, 2016

@VinInn we can organize all these steps once we have the changes propagated in DB.

@VinInn
Copy link
Contributor

VinInn commented Jun 1, 2016

@musich @ianna then we need the xml file, we can inspect (diff) it, maybe run 1000 events
and "sign off" the file content for the loading in DB and start proper validation

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 3d97c82 into cms-sw:CMSSW_8_1_X Jun 1, 2016
@kpedro88
Copy link
Contributor

kpedro88 commented Jun 2, 2016

When I try to run upgrade workflow 11000.0 in CMSSW_8_1_X_2016-06-02-1100, I get an error in step1:

%MSG-e SimG4CoreGeometry:  (NoModuleName) 02-Jun-2016 10:30:35 CDT pre-events
DDG4Builder::  material materials:micro_twisted_boundle_1 is not valid (in the DDD sense!)
%MSG
terminate called after throwing an instance of 'cms::Exception'
  what():  An exception of category 'SimG4CoreGeometry' occurred.
Exception Message:
 material is not valid from the Detector Description: materials:micro_twisted_boundle_1 

It looks like that material was introduced in this PR. Please fix ASAP.

@friccita
Copy link
Contributor Author

friccita commented Jun 2, 2016

@kpedro88 What is the geometry configuration loaded in workflow 11000.0?

@friccita
Copy link
Contributor Author

friccita commented Jun 2, 2016

@kpedro88 I see... cmsExtendedGeometry2023simXML_cfi.py loads these files Geometry/TrackerCommonData/data/PhaseI/pixbarladderfull*.xml for the pixel barrel layer definitions, which use the new materials defined for Phase I BPIX module cables. I will make another pull request to fix the PhaseII/materials.xml file so that it also includes these new materials from PhaseI.

@kpedro88
Copy link
Contributor

kpedro88 commented Jun 2, 2016

@friccita Great, thanks!

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