-
Notifications
You must be signed in to change notification settings - Fork 207
Add new "Mille" package to externals, update Millepede-II and GBL #10310
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
base: IB/CMSSW_16_1_X/master
Are you sure you want to change the base?
Conversation
|
cms-bot internal usage |
|
A new Pull Request was created by @goblirsc for branch IB/CMSSW_16_1_X/master. @akritkbehera, @cmsbuild, @iarspider, @raoatifshad, @smuzaffar can you please review it and eventually sign? Thanks. |
|
please test with cms-sw/cmssw#49963 |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b8b998/50978/summary.html The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
You can see more details here: Comparison SummarySummary:
Max Memory Comparisons exceeding threshold@cms-sw/core-l2 , I found 2 workflow step(s) with memory usage exceeding the error threshold: Expand to see workflows ...
|
|
|
||
| %prep | ||
| %setup -q -n %{n}-%{realversion} | ||
| grep -q 'CMAKE_CXX_STANDARD *11' cpp/CMakeLists.txt |
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.
@goblirsc , please do not delete this line. It is there to make sure that the cpp standard substitution we are doing in the next line works. I see that new gbl has default standard 17 ( https://gitlab.desy.de/millepede/general-broken-lines/-/blob/V04-00-00/cpp/CMakeLists.txt?ref_type=tags#L47) , so please add this line back and update the expected standard i.e.
grep -q 'CMAKE_CXX_STANDARD *17' cpp/CMakeLists.txt
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.
added lines back and in both cased specified 17 as the standard to replace
| -DCMAKE_VERBOSE_MAKEFILE=ON \ | ||
| -DEIGEN3_INCLUDE_DIR=${EIGEN_ROOT}/include/eigen3 \ | ||
| -DSUPPORT_ROOT=False \ | ||
| -DCMAKE_CXX_FLAGS="$CMS_EIGEN_CXX_FLAGS %{selected_microarch}" |
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.
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.
thanks - now done
mille.spec
Outdated
| rm -rf build | ||
| mkdir build | ||
| cd build | ||
| cmake \ | ||
| -DCMAKE_INSTALL_PREFIX=%{i} \ | ||
| ../ | ||
| make | ||
|
|
||
| %install | ||
| cd build | ||
| make install PREFIX=%{i} |
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.
| rm -rf build | |
| mkdir build | |
| cd build | |
| cmake \ | |
| -DCMAKE_INSTALL_PREFIX=%{i} \ | |
| ../ | |
| make | |
| %install | |
| cd build | |
| make install PREFIX=%{i} | |
| rm -rf ../build | |
| mkdir ../build | |
| cd ../build | |
| cmake \ | |
| DCMAKE_BUILD_TYPE=Release \ | |
| -DCMAKE_INSTALL_PREFIX=%{i} \ | |
| -DCMAKE_PREFIX_PATH="%{cmake_prefix_path}" | |
| ../%{n}-%{realversion} | |
| make %{makeprocesses} VERBOSE=1 | |
| %install | |
| cd ../build | |
| make install PREFIX=%{i} |
to avoid building in the source tree and also build in parallel & verbose modemake %{makeprocesses} VERBOSE=1
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.
thanks, done
millepede.spec
Outdated
| -DCMAKE_INSTALL_PREFIX=%{i} \ | ||
| -DLAPACK_OPENBLAS=off \ | ||
| ../ | ||
| make |
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.
same here, build outside the source tree i.e. try using ../build and also update cmake configuration to explicitly set CMAKE_PREFIX_PATH and CMAKE_BUILD_TYPE . Update make to run in parallel
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.
thanks - should be fixed in the latest commit
|
@goblirsc , couple of files also need relocation ( see https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b8b998/50978/external_checks/relocate/ ). so please update the specs to relocate these too |
| @@ -0,0 +1,25 @@ | |||
| ### RPM external mille V01-00-00 | |||
| ## INCLUDE cpp-standard | |||
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.
@goblirsc , if we do not want to override the c++ standard for mille then there is no need to have this line. The above line only sets the default c++std for cms software stack and packages which are sensitive to c++std can make use of it. So if you need to build mille for default cms c++std then keep this line but then also add -DCMAKE_CXX_STANDARD=%{cms_cxx_standard} to cmake
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.
thanks, now passing the cms_cxx_standard to CMake
|
Pull request #10310 was updated. |
|
please test with cms-sw/cmssw#49963 |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b8b998/51007/summary.html The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
You can see more details here: Comparison SummarySummary:
|
This PR updates the tracker alignment packages Millepede-II and GBL to use a new, common implementation of the "Mille" library responsible for writing and reading the alignment fit inputs to/from disk. The common Mille package is added as a new external package, and dependencies on it introduced to the Millepede-II and GBL spec files.
See slides for details on the new package.
Changes require updates to CMSSW, as some API calls change. These are in PR #49963.
Together, the changes pass the unit and runTheMatrix tests (few exceptions related to DAS / missing inputs, unrelated to PR).
In addition, changes were validated by running a full tracker alignment with the modified externals & CMSSW on lxplus8.