MTD geometry: update MTDGeometry test, use only DD4hep backend, add ETL pixel position test #46683
Conversation
|
cms-bot internal usage |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46683/42617 |
|
A new Pull Request was created by @fabiocos for master. It involves the following packages:
@Dr15Jones, @Moanwar, @bsunanda, @civanch, @cmsbuild, @kpedro88, @makortel, @mdhildreth, @srimanob, @subirsarkar can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test with cms-data/Geometry-TestReference#16 |
|
-1 Failed Tests: UnitTests 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: Unit TestsI found 1 errors in the following unit tests: ---> test TestDQMGUIUpload had ERRORS Comparison SummaryThere are some workflows for which there are errors in the baseline: Summary:
|
|
Failures appear unrelated to this PR |
cmsbuild
added
tests-pending
code-checks-pending
and removed
tests-rejected
requires-external
code-checks-approved
labels
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46683/42622 |
|
-1 Failed Tests: UnitTests 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: Unit TestsI found 1 errors in the following unit tests: ---> test TestDQMGUIUpload had ERRORS Comparison SummaryThere are some workflows for which there are errors in the baseline: Summary:
|
|
@cms-sw/geometry-l2 @cms-sw/upgrade-l2 the errors have nothing to do with this PR, any comment? |
|
+Upgrade |
|
@cms-sw/geometry-l2 comments? |
|
please test with cms-data/Geometry-TestReference#16 |
|
+1 Size: This PR adds an extra 12KB to repository Comparison SummarySummary:
|
|
+1 |
cmsbuild
added
geometry-approved
fully-signed
and removed
geometry-pending
pending-signatures
labels
|
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. @sextonkennedy, @antoniovilela, @rappoccio, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
|
I see that the unit test is failing in the IB, while it was successful in the PR test. I wonder whether it depends on details of the architecture, as this appears to be linked to rounding. I will fix that by reducing the number of digits used to the strict minimum needed |
Is the test failure seen only in MULTIARCHS IB? If it's only there, then this sounds like a x86-64-v2 vs v3 difference (FMA being the first suspect coming to mind); |
|
@makortel no, I see it also in aarch64. There was a similar issue in the |
PR description:
Complete revision of the (unit) test suite for
Geometry/MTDGeometryBuilder:MTDDigiGeometryAnalyzersimplified, no full dump of geometry;DD4hep_TestBTLPixelTopologymoved toDD4hep_TestPixelTopology, full comparison of ideal and reco geometries for both BTL and ETL, with dump of position of ETL reference pixel (needed to test the development of ETL v9).Only the DD4hep version of the test is now run, since
MTDGeometryis independent on the backend, provided theMTDNumberingBuilderoutput is the same.The filtering of volumes has been optimized to prevent the navigation of the full volume tree.
PR validation:
The code runs for scenario D110, providing the expected output. The reference needs to be updated in the
cms-dataarea to have a successful completion of the unit test.