Force a working BLAS kernel on the runners that need one - #897
Merged
Conversation
test_sdf_from_robot_with_scale_parameter went red on some runs of a commit and green on others, and the assertion said the signed distance field had been built on a grid 7.4 times too wide. Sampling one runner per job, 145 of them over five batches, split the result perfectly by CPU: every one of the ten Intel Xeon 6973P-C machines reproduced the failure to the last digit of both the resolution and the worst vertex, and none of the AMD EPYC 7763, 9V45, 9V74 or Xeon Platinum 8573C ones ever did. numpy's bundled OpenBLAS detects that CPU as Cooperlake, and the kernel it picks returns wrong numbers for (4, 4) by (4, n) dgemm once n passes about 128: most rows come back as [10*x + 10, 0, 0], the homogeneous row folded into the first column and the other two zeroed, 2311 of 2503 rows on the sample. That is the product trimesh.transform_points makes, so every mesh a URDF scales or places is built from it -- the collision mesh comes out a shape the bunny never had, SDFGen sizes the grid from its bounding box, and the failure surfaces four layers from the cause. The Mitsuba viewer sizing a Panda scene wrong in the same job was this too. Single threads make no difference, so it is not a race; einsum and the (3, 3) by (3, n) product are exact, so it is that kernel and that shape. HASWELL, SKYLAKEX and NEHALEM all give exact results and SAPPHIRERAPIDS does not, so force HASWELL -- the setting has to be harmless on the machines it is not aimed at, and SKYLAKEX would put AVX-512 kernels on the EPYC 7763s that are most of the pool and do not have them. Only on a runner that fails the multiply, so everything else keeps its own kernel. It cannot be done from inside the session, because OpenBLAS reads the variable when it loads. Confirmed on a batch of forty: five 6973P-C, five trips, forty correct grids. conftest runs the same check and stops with the reason and the command to set, so the same machine under someone's desk costs them a sentence rather than an afternoon. The test is left exactly as it was. It was right: the field really was wrong, and widening the tolerance to 4.19 grid cells would have taught it to accept a misshapen robot.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
test_sdf_from_robot_with_scale_parameterhas been going red on some runs of a commit and green on others, most recently on 15d396f, which produced one failing run and one clean one. The assertion said the signed distance field had been built on a grid 7.4 times too wide:It is not scipy 1.8.0, which is what the failing cells had in common by coincidence. In run 34554196186 the numpy-latest/scipy-1.8.0 cell passed while numpy-1.23.5/scipy-1.8.0 failed; in 34552744465 the numpy-latest/scipy-1.8.0 cell failed.
What it is
numpy's bundled OpenBLAS detects the
Intel(R) Xeon(R) 6973P-CasCore: Cooperlake, and that kernel returns wrong results for(4, 4)by(4, n)double-precision products oncenpasses roughly 128. Most rows come back as[10*x + 10, 0, 0]: the homogeneous row folded into the first column and the other two zeroed. 2311 of 2503 rows on the sample.That is the product
trimesh.transform_pointsmakes, so every mesh a URDF scales or places is built from it. The collision mesh comes out a shape the bunny never had, SDFGen sizes the grid from its bounding box, and the failure surfaces four layers from the cause. The Mitsuba viewer sizing a Panda scene wrong in the same job was this too.Minimal reproducer:
How it was pinned down
One runner per job, 145 jobs over five batches, printing the diagnostics whether or not the assertion held, because a machine can carry the defect and still pass: an over-wide grid on its own only moves the worst vertex to about half a cell, measured at four grid sizes.
On an affected runner:
np.dot(2503, 3) x (3, 3)OPENBLAS_NUM_THREADS=1OPENBLAS_CORETYPE=HASWELLOPENBLAS_CORETYPE=SKYLAKEXOPENBLAS_CORETYPE=NEHALEMOPENBLAS_CORETYPE=SAPPHIRERAPIDSSingle threads make no difference, so it is not a race. einsum and the
(3, 3)product are exact, so it is that kernel and that shape.What this changes
The workflow checks that product after installing numpy and scipy, and sets
OPENBLAS_CORETYPE=HASWELLonly on a runner that fails it, with a warning annotation naming the CPU. Haswell rather than SkylakeX because the setting has to be harmless on the machines it is not aimed at, and the EPYC 7763s that are most of the pool have no AVX-512. Healthy runners keep their own kernels. It cannot be done from inside the session: OpenBLAS reads the variable when it loads.Confirmed end to end on a batch of forty runners: five were 6973P-C, the guard tripped on exactly those five, and all forty then produced the correct grid.
tests/conftest.pyruns the same check at session start and stops with the reason and the command to set, so the same machine under someone's desk costs a sentence rather than an afternoon.What this does not change
Link.collision_mesharound the matrix product would only make a broken machine look like it was working.Not reported upstream. The closest existing reports are OpenBLAS #2168, #3454, #5267 and #2769; none names this CPU.