| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
This removes the NPY_DISABLE_CPU_FEATURES flag from the sphinx and tk tests as they emit warnings on CI which leads to failure from the subprocess. These don't need to be disabled on these tests, so remove them from the environment variables that are passed in.
|
This is pretty obscure. Does it make sense to just up the test tolerance instead? Though I think a couple of tests were quite wrong. Why did these flags have such a large effect? |
Sorry, something went wrong.
I don't think any were unexpectedly wrong. I downloaded the failed images and all were minor chnages apart from errorbar_mixed, which was a large change because (I presume) a small difference was causing the axes limit to jump from 1e-2 to 1e-3. I think this is the most pragmatic way to go forward, without having to play whack-a-mole with test tolerances across the test suite. |
Sorry, something went wrong.
|
Oh I have the opposite point of view. If we have tests that are susceptible to compiler foibles maybe we should fix the tests or increase their tolerance, because there will always be more compiler foibles. |
Sorry, something went wrong.
|
I agree this is a pretty big hammer, saying we won't test against certain floating point instruction sets. However, I assume we rely on numpy for some verification that their floating point calculations are "good enough" for them and this is impacting us because we are doing pixel-perfect comparisons with floating point inputs. I can see both arguments here, so I'm not sure which one people would be more generally comfortable with (increased tolerances, or reduced instruction sets). There are only a few tests failing, so the tolerances on a few of them could probably be bumped, but the errorbar one we should probably update the autoscaling in that test by just bumping the np.minimum() call to add in a small epsilon rather than adding a large tolerance. |
Sorry, something went wrong.
|
We have to live with the fact that our dependencies may introduce minor variations. First and foremost we don't want brittle tests. There are two ways to get there:
I argue that if we can easily get away with (1) that's better than (2). We have more control and don't blindly accept other changes as well. For the same reason we pin freetype or remove text overall. |
Sorry, something went wrong.
|
I merge as is to unbreak the builds. @jklymak if you think tolerances are the better approach we can discuss this in the next call, and then maybe change. |
Sorry, something went wrong.
|
Sure, I've added to the call agenda. |
Sorry, something went wrong.
…099-on-v3.5.x Backport PR #22099 on branch v3.5.x (CI: Disable numpy avx512 instructions)
|
I may have over-learned from our issue we had with "rendering the wrong glyph but tests are passing", but I am very very concerned about bumping tolerances in anything but a per-test basis. |
Sorry, something went wrong.
|
Sure, I wasn't suggesting a general large tolerance. But here we have one test that had a large change because of floating point slop. So in my opinion that test should be made more robust. The other changes are pretty small, so upping those tolerances would also make sense to me. |
Sorry, something went wrong.
|
Here is a link to a commit that would be what @jklymak suggests to increase tolerances/dtypes. All of them were pretty small. |
Sorry, something went wrong.
|
I came across https://etna.math.kent.edu/vol.52.2020/pp358-369.dir/pp358-369.pdf recently which has a case where increasing the precision of floats can make things worse (sometimes the rounding is actually in your favor!). |
Sorry, something went wrong.
|
@greglucas can you open a PR with those tolerances, they seem reasonable to me. |
Sorry, something went wrong.
|
Sure, see #22132. That is an interesting finding! I didn't think about trying single precision instead in the update... |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
PR Summary
This disables the AVX512 instruction set at runtime within the CI using the NPY_DISABLE_CPU_FEATURES environment variable. This is due to small floating point differences causing test failures when using that instruction set on numpy 1.22 wheels on the GitHub Actions runners.
PR Checklist
Tests and Styling
Documentation