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
DD4HEP GEM migration #28796
DD4HEP GEM migration #28796
Conversation
The code-checks are being triggered in jenkins. |
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28796/13484
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
The code-checks are being triggered in jenkins. |
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28796/13485
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
The code-checks are being triggered in jenkins. |
1 similar comment
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28796/13487
|
A new Pull Request was created by @slomeo (Sergio Lo Meo) for master. It involves the following packages: DetectorDescription/DDCMS The following packages do not have a category, yet: cmsRecoGeom-2021.root @civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @kpedro88 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, exclude the root file from this PR. Files as such can be added to the data repository if necessary.
#include "Geometry/MuonNumbering/interface/DD4hep_MuonNumbering.h" | ||
#include "Geometry/MuonNumbering/interface/MuonDDDNumbering.h" | ||
#include "Geometry/MuonNumbering/interface/MuonBaseNumber.h" | ||
#include <DetectorDescription/DDCMS/interface/DDCompactView.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slomeo - please, use quotes "" instead of <>
<library file="GEMGeometryESModule.cc"> | ||
<flags EDM_PLUGIN="1"/> | ||
</library> | ||
<library file="ME0GeometryESModule.cc"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slomeo - the ME0 geometry builder is dropped... Please, restore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ianna There is a problem if I restore the ME0 Geometry Builder in Geometry/GEMGeometryBuilder/plugins/BuildFile.xml even if I made a "scram b clean" and then a "scram b -j 10".
The messages I see is:
....
Building shared library tmp/slc7_amd64_gcc820/src/DetectorDescription/DDCMS/plugins/DetectorDescriptionDD4HepPlugins/libDetectorDescriptionDD4HepPlugins.so
/cvmfs/cms-ib.cern.ch/nweek-02613/slc7_amd64_gcc820/external/gcc/8.2.0-pafccj/bin/../lib/gcc/x86_64-unknown-linux-gnu/8.3.1/../../../../x86_64-unknown-linux-gnu/bin/ld: tmp/slc7_amd64_gcc820/src/Geometry/GEMGeometryBuilder/plugins/ME0GeometryESModule/ME0GeometryESModule.cc.o: in functionME0GeometryESModule::produce(MuonGeometryRecord const&)': ME0GeometryESModule.cc:(.text+0x3f7): undefined reference to
ME0GeometryBuilderFromDDD::ME0GeometryBuilderFromDDD()'
/cvmfs/cms-ib.cern.ch/nweek-02613/slc7_amd64_gcc820/external/gcc/8.2.0-pafccj/bin/../lib/gcc/x86_64-unknown-linux-gnu/8.3.1/../../../../x86_64-unknown-linux-gnu/bin/ld: ME0GeometryESModule.cc:(.text+0x413): undefined reference toME0GeometryBuilderFromDDD::build(DDCompactView const*, MuonDDDConstants const&)' /cvmfs/cms-ib.cern.ch/nweek-02613/slc7_amd64_gcc820/external/gcc/8.2.0-pafccj/bin/../lib/gcc/x86_64-unknown-linux-gnu/8.3.1/../../../../x86_64-unknown-linux-gnu/bin/ld: ME0GeometryESModule.cc:(.text+0x41e): undefined reference to
ME0GeometryBuilderFromDDD::~ME0GeometryBuilderFromDDD()'
/cvmfs/cms-ib.cern.ch/nweek-02613/slc7_amd64_gcc820/external/gcc/8.2.0-pafccj/bin/../lib/gcc/x86_64-unknown-linux-gnu/8.3.1/../../../../x86_64-unknown-linux-gnu/bin/ld: ME0GeometryESModule.cc:(.text+0x5fa): undefined reference toME0GeometryBuilderFromCondDB::ME0GeometryBuilderFromCondDB()' /cvmfs/cms-ib.cern.ch/nweek-02613/slc7_amd64_gcc820/external/gcc/8.2.0-pafccj/bin/../lib/gcc/x86_64-unknown-linux-gnu/8.3.1/../../../../x86_64-unknown-linux-gnu/bin/ld: ME0GeometryESModule.cc:(.text+0x605): undefined reference to
ME0GeometryBuilderFromCondDB::build(RecoIdealGeometry const&)'
/cvmfs/cms-ib.cern.ch/nweek-02613/slc7_amd64_gcc820/external/gcc/8.2.0-pafccj/bin/../lib/gcc/x86_64-unknown-linux-gnu/8.3.1/../../../../x86_64-unknown-linux-gnu/bin/ld: ME0GeometryESModule.cc:(.text+0x610): undefined reference toME0GeometryBuilderFromCondDB::~ME0GeometryBuilderFromCondDB()' /cvmfs/cms-ib.cern.ch/nweek-02613/slc7_amd64_gcc820/external/gcc/8.2.0-pafccj/bin/../lib/gcc/x86_64-unknown-linux-gnu/8.3.1/../../../../x86_64-unknown-linux-gnu/bin/ld: tmp/slc7_amd64_gcc820/src/Geometry/GEMGeometryBuilder/plugins/ME0GeometryESModule/ME0GeometryESModule.cc.o: in function
ME0GeometryESModule::produce(MuonGeometryRecord const&) [clone .cold.341]':
ME0GeometryESModule.cc:(.text.unlikely+0x31c): undefined reference toME0GeometryBuilderFromDDD::~ME0GeometryBuilderFromDDD()' /cvmfs/cms-ib.cern.ch/nweek-02613/slc7_amd64_gcc820/external/gcc/8.2.0-pafccj/bin/../lib/gcc/x86_64-unknown-linux-gnu/8.3.1/../../../../x86_64-unknown-linux-gnu/bin/ld: ME0GeometryESModule.cc:(.text.unlikely+0x368): undefined reference to
ME0GeometryBuilderFromCondDB::~ME0GeometryBuilderFromCondDB()'
/cvmfs/cms-ib.cern.ch/nweek-02613/slc7_amd64_gcc820/external/gcc/8.2.0-pafccj/bin/../lib/gcc/x86_64-unknown-linux-gnu/8.3.1/../../../../x86_64-unknown-linux-gnu/bin/ld: tmp/slc7_amd64_gcc820/src/Geometry/GEMGeometryBuilder/plugins/ME0GeometryESModule/ME0GeometryESModule.cc.o: in functionedm::eventsetup::ProxyArgumentFactoryTemplate<edm::eventsetup::CallbackProxy<edm::eventsetup::Callback<ME0GeometryESModule, std::unique_ptr<ME0Geometry, std::default_delete<ME0Geometry> >, MuonGeometryRecord, edm::eventsetup::CallbackSimpleDecorator<MuonGeometryRecord> >, MuonGeometryRecord, std::unique_ptr<ME0Geometry, std::default_delete<ME0Geometry> > >, edm::eventsetup::Callback<ME0GeometryESModule, std::unique_ptr<ME0Geometry, std::default_delete<ME0Geometry> >, MuonGeometryRecord, edm::eventsetup::CallbackSimpleDecorator<MuonGeometryRecord> > >::makeKey(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const': ME0GeometryESModule.cc:(.text._ZNK3edm10eventsetup28ProxyArgumentFactoryTemplateINS0_13CallbackProxyINS0_8CallbackI19ME0GeometryESModuleSt10unique_ptrI11ME0GeometrySt14default_deleteIS6_EE18MuonGeometryRecordNS0_23CallbackSimpleDecoratorISA_EEEESA_S9_EESD_E7makeKeyERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE[_ZNK3edm10eventsetup28ProxyArgumentFactoryTemplateINS0_13CallbackProxyINS0_8CallbackI19ME0GeometryESModuleSt10unique_ptrI11ME0GeometrySt14default_deleteIS6_EE18MuonGeometryRecordNS0_23CallbackSimpleDecoratorISA_EEEESA_S9_EESD_E7makeKeyERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE]+0xd): undefined reference to
char const* edm::typelookup::className()'
/cvmfs/cms-ib.cern.ch/nweek-02613/slc7_amd64_gcc820/external/gcc/8.2.0-pafccj/bin/../lib/gcc/x86_64-unknown-linux-gnu/8.3.1/../../../../x86_64-unknown-linux-gnu/bin/ld: ME0GeometryESModule.cc:(.text._ZNK3edm10eventsetup28ProxyArgumentFactoryTemplateINS0_13CallbackProxyINS0_8CallbackI19ME0GeometryESModuleSt10unique_ptrI11ME0GeometrySt14default_deleteIS6_EE18MuonGeometryRecordNS0_23CallbackSimpleDecoratorISA_EEEESA_S9_EESD_E7makeKeyERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE[_ZNK3edm10eventsetup28ProxyArgumentFactoryTemplateINS0_13CallbackProxyINS0_8CallbackI19ME0GeometryESModuleSt10unique_ptrI11ME0GeometrySt14default_deleteIS6_EE18MuonGeometryRecordNS0_23CallbackSimpleDecoratorISA_EEEESA_S9_EESD_E7makeKeyERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE]+0x16): undefined reference to `std::type_info const& edm::typelookup::classTypeInfo()'
collect2: error: ld returned 1 exit status
gmake: *** [config/SCRAM/GMake/Makefile.rules:1742: tmp/slc7_amd64_gcc820/src/Geometry/GEMGeometryBuilder/plugins/ME0GeometryESModule/libME0GeometryESModule.so] Error 1
gmake: *** Waiting for unfinished jobs....
Copying tmp/slc7_amd64_gcc820/src/Geometry/HcalCommonData/src/GeometryHcalCommonData/libGeometryHcalCommonData.so to productstore area:
Copying tmp/slc7_amd64_gcc820/src/Geometry/HGCalCommonData/src/GeometryHGCalCommonData/libGeometryHGCalCommonData.so to productstore area:
Leaving library rule at src/DetectorDescription/DDCMS/plugins
Leaving library rule at src/DetectorDescription/DDCMS/plugins
Leaving library rule at Geometry/HGCalCommonData
Leaving library rule at Geometry/HcalCommonData
Leaving library rule at src/DetectorDescription/DDCMS/plugins
--- Registered EDM Plugin: RPCGeometryValidationPlugins
--- Registered EDM Plugin: DTGeometryValidationPlugins
gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is because you do not link it against Geometry/GEMGeometryBuilder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ianna : done
The tests are being triggered in jenkins. |
+1 |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
+upgrade |
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, @silviodonato, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
Looks like this PR is the reason behind the massive RelVal errors in IBs. [a]
|
@smuzaffar I can't understand how this PR could have broken the IB, in configuration side it only adds two test configuration files. #28847 must be somehow involved since it moved the I see now that #28847 indeed missed to migrate two cfi files. Also the CMSSW_11_1_X_2020-02-06-2300 seems to be the first full build since #28847 got merged, so could it be that the old (anyway, I can make a quick fix) |
@makortel @smuzaffar yes, unfortunately I missed to update two HGCal fragments in my migration, sorry for this. I see that #28881 fixes this, @silviodonato as this is trivial and straightforward please just merge it for next IB. As far as I can see in https://cmssdt.cern.ch/lxr/search?%21v=CMSSW_11_1_X_2020-02-05-2300&_filestring=&_string=dd4hep_cff I would say that should be enough. |
@slomeo @fabiocos this PR is also the reason of the failure of the following unit test I checked with
|
@silviodonato yes, I can confirm (and there is nothing else touching the geometry in the IB that might justify the failure). Looking at the PR the reason is not immediately obvious to me. |
ok, I believe the cause is a change in the output of the dump after this PR, that causes the compariosn with the reference log file to fail. It should be checked, and in case the log file replaced. |
@@ -29,7 +29,7 @@ void MuonBaseNumber::addBase(const LevelBaseNumber& num) { | |||
} | |||
sortedBaseNumber.insert(cur, num); | |||
|
|||
#ifdef LOCAL_DEBUG | |||
//#ifdef LOCAL_DEBUG |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slomeo @cvuosalo @silviodonato this update of MuonBaseNumber
adds lots of (I guess) unwanted printouts, this changes the log file (but if LogInfo
is silenced this is not noticed but in the summary), and as the output is different wrt the reference, the testDTGeometry unit test fails. @slomeo could you please fix this? Why removing the ifdef
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR description:
DD4HEP GEM migration
PR validation:
by validateGEMGeometry_cfg.py script (see pdf attached)
GEM1.pdf
GEM2.pdf
GEM3.pdf
GEM4.pdf
GEM5.pdf
GEM6.pdf
GEM7.pdf
GEM8.pdf
if this PR is a backport please specify the original PR:
Before submitting your pull requests, make sure you followed this checklist: