Fix failing test on ARM by rounding small values to 0 #29052
Conversation
|
The code-checks are being triggered in jenkins. |
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29052/13922
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-29052/13923
|
|
A new Pull Request was created by @cvuosalo (Carl Vuosalo) for master. It involves the following packages: DataFormats/Math @perrotta, @civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
+1 |
|
Comparison job queued. |
|
please test for slc7_aarch64_gcc820 |
cmsbuild
added
comparison-pending
tests-pending
and removed
comparison-available
tests-approved
labels
|
The tests are being triggered in jenkins.
|
|
+1 |
|
Comparison job queued. |
|
+1 |
|
@ianna It looks like you changed your mind about generating the reference file on-the-fly. Could you please delete your comment about it, or add a comment saying so? I wouldn't want any reviewers to be confused. |
|
@cvuosalo - IMHO, the reference file should be generated on the fly, but ultimately, it's your decision how high this is on the priority list. |
|
type bugfix |
|
@ianna I will leave the PR as it is for now. I think on-the-fly generation of the reference file is lower priority than other issues I need to work on. |
|
+1
|
cmsbuild
added
fully-signed
reconstruction-approved
and removed
pending-signatures
reconstruction-pending
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. @davidlange6, @silviodonato, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
Certain units tests are failing on the ARM architecture because tiny floating-point values near 0 are slightly different than on the standard CPU, as described in issue #29036.
To fix this problem, the printed values used for comparison will be rounded to 0 if they are less than 1e-7. The internal values will not be changed, only the printed values used to compare the test results and the baseline reference file, which is being updated.
This change means the DT Geometry unit test will be less sensitive to small differences in norm vector values near zero, which could possibly mean it might miss a real problem, but the current test is too sensitive in failing because of 1e-16 fluctuations. I think the version of the test in this PR is sufficiently sensitive to catch real errors while avoiding false positives.
This PR adds a new utility file
Rounding.h. I looked to see if a function to round values near 0 already exists, but I couldn't find one, so I added this new file. If there is already such a rounding function, or if I put the new file in the wrong place, this PR can be revised.PR validation:
The unit test should now be platform-independent. I hope this PR can be tested against the ARM IB because I have not be able to test the code change on an ARM machine.
No backport is needed.