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
MTD geometry: fix BTL numbering scheme for DD4hep+Geant4 extra level in path #39843
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39843/32724
|
A new Pull Request was created by @fabiocos (Fabio Cossutti) for master. It involves the following packages:
@civanch, @Dr15Jones, @bsunanda, @makortel, @ianna, @mdhildreth, @cmsbuild, @AdrianoDee, @srimanob, @clacaputo, @mandrenguyen can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
test parameters:
|
please test |
-1 Failed Tests: RelVals RelVals-INPUT RelVals
RelVals-INPUT
Expand to see more relval errors ...
|
urgent |
@perrotta it seems to me that this test failure is unrelated to this PR |
Hi @fabiocos Three additional workflows are not tested. I don't see them in https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ef1843/28490/runTheMatrix-results/ |
@cmsbuild please test Retrigger the test |
-1 Failed Tests: RelVals RelVals |
I used it for my own private tests, but of course using |
test parameters:
|
please test |
@perrotta , cms-sw/cms-bot#1871 should now properly report such errors too |
@smuzaffar thanks, this is generally useful for the future |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ef1843/28514/summary.html Comparison SummaryThere are some workflows for which there are errors in the baseline: Summary:
|
comparison failures look unrelated to this PR |
@cms-sw/geometry-l2 @cms-sw/reconstruction-l2 @cms-sw/upgrade-l2 could you please review this PR? |
+reconstruction |
+Upgrade |
+geometry |
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. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
This PR provide a fix for the crash observed in wf 20834.911 after the integration of #39670 . The use of DD4hep within Geant4 adds effectively one more level in the volume stack, which breaks the logic of the BTL numbering scheme as updated. For this reason an extra protection is added, to address this specific case.
At the same time also the static analyser issue #39670 (review) is addressed.
PR validation:
Test workflows 20834.0, 20834.911 and 23634.911 are correctly executed. Unit tests pass.