Closed metab0t closed 1 year ago
Hi! This is the friendly automated conda-forge-linting service.
I just wanted to let you know that I linted all conda-recipes in your PR (recipe
) and found it was in an excellent condition.
The failed tests on Linux are caused by +0.0 != -0.0
.
It seems to be a numerical issue and too strict equality criteria.
@esc How can we disable some tests in run_tests.sh (python -m numba.tests.test_runtests
) ?
We are seeing the following test failures. AFAICT there are small things like -0
that throw the comparison
@stuartarchibald how should we proceed here?
If it is just a small numeric error, can tests be disabled for this version (it seems the package builds fine and tests again almost all tests except those small numeric values)? They can be reenabled for the next version when it is fixed in Numba.
It's my understanding the Numba team is currently investigating this. So let's give them a little time to follow up
Is there any new on this one?
@jakirkham apologies for the delay on this. I've debugged it and think that the issue is actually rooted in a problem in the CF docker container glibc
/libm
. This program demonstrates:
#include <stdio.h>
#include <complex.h>
int main(void) {
double complex x = -0.0 - 1.0 * I;
double complex result;
printf("Input : (%18.16f, %18.16f)\n", creal(x), cimag(x));
result = casinh(x);
printf("Result: (%18.16f, %18.16f)\n", creal(result), cimag(result));
return 0;
}
which, in the container used for the build, produces:
$ $CC issue128.c -o issue128 -lm
$ ./issue128
Input : (-0.0000000000000000, -1.0000000000000000)
Result: (0.0000000000000000, -1.5707963267948966)
Note that the real part of the result does not copy the sign of the zero from the input. Further, running the failing test in the same container under gdb
with a breakpoint set on casinh
confirms it's being hit by NumPy and is likely the cause of the mismatch (Numba's implementation is largely internal and IIRC doesn't rely on the casinh
symbol, so should just behave consistently invariant of libm
's complex function implementations).
This issue was discussed at the Numba maintainer's triage meeting on 2023-10-31 and the conclusion was that if the issue is indeed in libm
, then, given the way in which this part of the test suite is written, it would be easiest for Conda-Forge to carry a "skip these tests" style patch.
As an aside, as part of the above discussion, it wasn't clear why the containers are using glibc
of version 2.10 and 2.12? I did try and see if there was a ticket tracking the potential libm
issue but couldn't find an issue tracker or source that would go back that far (seems to be from around 2015?).
Hope this helps?
@jakirkham OK to merge?
Can we just skip the broken tests? Skipping all tests on Linux sounds like it could hide future problems.
The test system of numba is based on unittests. It is easy to select a fraction of tests but I don't know how to filter specific tests like pytest.
The test system of numba is based on unittests. It is easy to select a fraction of tests but I don't know how to filter specific tests like pytest.
Numba has a number of convenience functions for skipping specific tests, like this one:
https://github.com/numba/numba/blob/main/numba/tests/support.py#L92
Here is the documentation about how to skip things using unittest
:
https://docs.python.org/3/library/unittest.html#skipping-tests-and-expected-failures
So, probably a decorator like:
@unittest.skipIf(sys.platform.startswith('linux'), "Skipping on linux for conda-forge, see issue #128")
I don't have the expertise or bandwidth to dig into which parts of tests should be precisely skipped. Now I use llvmlite from conda-forge channel and numba from numba channel as a temporary solution.
@conda-forge-admin, please rerender
As an aside, as part of the above discussion, it wasn't clear why the containers are using
glibc
of version 2.10 and 2.12? I did try and see if there was a ticket tracking the potentiallibm
issue but couldn't find an issue tracker or source that would go back that far (seems to be from around 2015?).
While Anaconda stopped building for CentOS 6 (glibc 2.12) in 2021 after its regular maintenance EOL, conda-forge continues to build for CentOS 6 by default until June next year (RHEL 6 extended support EOL). We do, however started to only test by default against CentOS 7 (glibc 2.17) about 2 years ago. For a timeline see:
Due to slight misunderstanding this feedstock continued to be tested against CentOS 6 (we hadn't yet merged a commit to rectify that).
That being said, apparently we have slight regression in NumPy 1.26 regarding arctanh
on glibc<=2.17
:
python -c 'import numpy as np; print("pass" if np.signbit(np.arctanh(-1j).real) else "fail", np.__version__)'
On CentOS 7:
/usr/lib64/libc.so.6 --version | head -n1
GNU C Library (GNU libc) stable release version 2.17, by Roland McGrath et al.
pass 1.25.2
fail 1.26.0
On Ubuntu 14.04:
/lib/x86_64-linux-gnu/libc-2.19.so --version | head -n1
GNU C Library (Ubuntu EGLIBC 2.19-0ubuntu6.15) stable release version 2.19, by Roland McGrath et al.
pass 1.25.2
pass 1.26.0
Small reproducer showing different behavior in glibc versions like the one given by @stuartarchibald in https://github.com/conda-forge/numba-feedstock/pull/128#issuecomment-1790592852 :
{ cat << 'EOF'
#include <stdio.h>
#include <complex.h>
#include <math.h>
void main(void) {
double complex x = -I;
if (signbit(creal(catanh(x))))
printf("pass\n");
else
printf("fail\n");
}
EOF
} | $CC -x c - -o issue128 -lm
CentOS 7:
# ./issue128
fail
Ubuntu 14.04:
# ./issue128
pass
^-- If anyone wants to report that upstream to NumPy, feel free to do so.
^-- If anyone wants to report that upstream to NumPy, feel free to do so.
I haven't yet opened an upstream issue but put out https://github.com/conda-forge/numpy-feedstock/pull/304 for further inspection/reference in future upstream issue.
@conda-forge/numba : Anyone opposed to merging?
I added the very specific numpy !=1.26.0 # [linux and x86_64]
to the test requirements so that this will be able to test for numpy>1.26.0
builds in the future.
I haven't yet opened an upstream issue but put out conda-forge/numpy-feedstock#304 for further inspection/reference in future upstream issue.
Upstream issue: https://github.com/numpy/numpy/issues/25087
I've also opened an issue for Numba to test against glibc=2.17 upstream so that we could react to upstream NumPy changes earlier in the future.
xref:
Thanks Stuart for digging into the test issue and following up! 🙏 Agree that we can just ignore those tests
Thanks everyone (especially Marcel) on bringing this up-to-date! 🙏
@beckermr , could you please read over the discussion and changes here? Would appreciate hearing your thoughts (as I think you discovered the GLIBC symbol leak issue previously)
(as I think you discovered the GLIBC symbol leak issue previously)
That was a false positive coming from defaults
, see https://github.com/conda-forge/numba-feedstock/issues/89#issuecomment-1323405834 .
This feedstock is build with cos6 because when it was built against cos7, it started to carry cos7 symbols and not just the cos6 ones.
Ahhh thanks @mbargull! We all missed that apparently. LGTM!
Hi! This is the friendly conda-forge automerge bot!
Commits were made to this PR after the automerge
label was added. For security reasons, I have disabled automerge by removing the automerge
label. Please add the automerge
label again (or ask a maintainer to do so) if you'd like to enable automerge again!
I haven't yet opened an upstream issue but put out conda-forge/numpy-feedstock#304 for further inspection/reference in future upstream issue.
Upstream issue: numpy/numpy#25087
Upstream PR to fix it: https://github.com/numpy/numpy/pull/25093
Thanks for all your efforts @mbargull @jakirkham, great to see this shipped!
Checklist
0
(if the version changed)conda-smithy
(Use the phrase code>@<space/conda-forge-admin, please rerender in a comment in this PR for automated rerendering)