-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Update tracker alignment algs to new Millepede+GBL versions #49963
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: master
Are you sure you want to change the base?
Conversation
|
cms-bot internal usage |
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49963/47743 ERROR: Build errors found during clang-tidy run. |
|
code-checks with cms.week0.PR_6e7d7ce1/100.0-e18971569e0e45b7e8c26c8607cf3bf8 |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49963/47759 |
|
A new Pull Request was created by @goblirsc for master. It involves the following packages:
@Alejandro1400, @JanChyczynski, @arunhep, @atpathak, @cmsbuild, @perrotta, @tvami can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
test parameters: |
|
please test |
| @@ -1581,17 +1595,11 @@ void MillePedeAlignmentAlgorithm::addLasBeam(const EventInfo &eventInfo, | |||
| const float residual = hit.localPosition().x() - tsoses[iHit].localPosition().x(); | |||
| // error from file or assume 0.003 | |||
| const float error = 0.003; // hit.localPositionError().xx(); sqrt??? | |||
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 beyond this PR, but I do wonder: is this really meant to be hardcoded at 0.003?
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.
as this pertains to MillePedeAlignmentAlgorithm::addLasBeam (LAS system decomissioned in 2014) I hardly can see how this could be relevant.
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.
I didnt realize this was for LAS. So this function could be fully removed too then?
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.
probably 🤷♂️
|
+1 Size: This PR adds an extra 16KB to repository 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:
|
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49963/47833 |
|
Pull request #49963 was updated. @Alejandro1400, @JanChyczynski, @arunhep, @atpathak, @cmsbuild, @perrotta, @tvami can you please check and sign again. |
|
@goblirsc probably before launching the tests it could be worthwhile to squash all commits into one. |
adb73be to
2dca55b
Compare
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49963/47839 |
|
Pull request #49963 was updated. @Alejandro1400, @JanChyczynski, @arunhep, @atpathak, @cmsbuild, @perrotta, @tvami can you please check and sign again. |
|
please test |
|
+1 Size: This PR adds an extra 36KB to repository 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:
|
|
+alca
|
|
+1 |
|
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. @mandrenguyen, @sextonkennedy, @ftenchini (and backports should be raised in the release meeting by the corresponding L2) |
|
REMINDER @mandrenguyen, @sextonkennedy, @ftenchini: This PR was tested with cms-sw/cmsdist#10310, please check if they should be merged together |
PR description:
This PR updates the CMSSW client code for interacting with Millepede2 and GBL to accomodate the latest updates to said packages.
Millebinary file interface.Milleclass.gbl::MilleRecordinstance in MillepedeAlignmentAlgorithm simultaneously talking to the same file as Mille instance (fragile).Dependencies / expected changes / documentation:
PR validation:
scram b runtests use-ibeos): passingrunTheMatrix.py -l limited -i all --ibeo: all tests passing except for few failures unrelated to the PR (missing datasets for e/gamma 136.X, DAS errors for 2022.00X-2025.00X)If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:
No backport needed
Before submitting your pull requests, make sure you followed this checklist:
✔️ verify that the PR is really intended for the chosen branch
✔️ verify that changes follow CMS Naming, Coding, And Style Rules
✔️ verify that the PR passes the basic test procedure suggested in the CMSSW PR instructions