-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
62X SLHC GEMGeometryBuilder cleanup #4201
62X SLHC GEMGeometryBuilder cleanup #4201
Conversation
A new Pull Request was created by @dildick (Sven Dildick) for CMSSW_6_2_X_SLHC. 62X SLHC GEMGeometryBuilder cleanup It involves the following packages: Configuration/Geometry @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @nclopezo, @Degano, @ktf can you please review it and eventually sign? Thanks. |
@dildick - you need to update the new (2 years old) python fragments in GEMGeometryBuilder so that they would match the old ones in GEMGeometry. Yana |
If you are referring to the parameter "compatibiltyWith11". That one is untracked and true in https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_X_SLHC/Geometry/GEMGeometryBuilder/plugins/GEMGeometryESModule.cc and https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_X_SLHC/Geometry/GEMGeometryBuilder/plugins/ME0GeometryESModule.cc. The configs in https://github.com/cms-sw/cmssw/tree/CMSSW_6_2_X_SLHC/Geometry/GEMGeometry/python never reinitialized compatibiltyWith11, so compatibiltyWith11 was (and is) always true. In all, there is no net change. |
@dildick - yes. Could you, please, remove it from python fragments? Thanks. |
Ok. Done. Actually, I would like to go even 1 step further, namely remove it all together from the code. AFAICT, that parameter is not even used... (do a search for theComp11Flag in https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_X_SLHC/Geometry/GEMGeometryBuilder/src/GEMGeometryBuilderFromDDD.cc) |
Pull request #4201 was updated. @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @nclopezo, @Degano, @ktf can you please check and sign again. |
Pull request #4201 was updated. @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @nclopezo, @Degano, @ktf can you please check and sign again. |
Pull request #4201 was updated. @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @nclopezo, @Degano, @ktf can you please check and sign again. |
+1 |
GEMGeometryParsFromDD is used in CondTools/Geometry/plugins/GEMRecoIdealDBLoader.cc. It either needs to be put back or taken out of GEMRecoIdealDBLoader. |
This pull request is fully signed and it will be integrated in one of the next CMSSW_6_2_X_SLHC IBs unless changes or unless it breaks tests. @fratnikov, @mark-grimes can you please take care of it? |
merge Just tested with 4 muons. Initially, besides the expected failures: 12000 failed in step 3 with a root error I've seen before which is intermittent:
11400 failed in step 4:
Reran both tests and they passed. No idea what the original problems were but they look unrelated to the pull request. Something to look out for. |
Pull request #4201 was updated. @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @nclopezo, @Degano, @ktf can you please check and sign again. |
62X SLHC GEMGeometryBuilder cleanup
(https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideUpgradeGeom#Extended2023TTI)
EDIT: