sgkit-dev / sgkit

Scalable genetics toolkit
https://sgkit-dev.github.io/sgkit
Apache License 2.0
235 stars 32 forks source link

Run tests against NumPy 2 #1234

Closed tomwhite closed 2 months ago

tomwhite commented 5 months ago

Tim made some fixes in #1228, but there are still some more

tomwhite commented 5 months ago

... the tests against scikit-allel are still failing. Maybe time to add the scikit-allel test data and drop the dependency?

Originally posted by @timothymillar in https://github.com/sgkit-dev/sgkit/issues/1228#issuecomment-2178140077

I didn't see this before, but I agree - it looks like scikit-allel won't work against NumPy 2 since its cython code is incompatible.

We only use it in the tests for comparing our methods implementations:

As @timothymillar said, we could export some test data from scikit-allel for the few remaining cases where we want to check the output of sgkit.

tomwhite commented 5 months ago

Another failing case I found was in the VCF writer code.

``` ================================================================================= FAILURES ================================================================================== __________________________________________________________________________ test_ftoa[0.0005-0.001] __________________________________________________________________________ f = np.float32(0.0005), a = '0.001' @pytest.mark.parametrize( "f, a", [ (0.0, "0"), (0.0001, "0"), (0.0005, "0.001"), (0.3, "0.3"), (0.32, "0.32"), (0.329, "0.329"), (0.3217, "0.322"), (8.0, "8"), (8.0001, "8"), (8.3, "8.3"), (8.32, "8.32"), (8.329, "8.329"), (8.3217, "8.322"), (443.998, "443.998"), (1028.0, "1028"), (1028.0001, "1028"), (1028.3, "1028.3"), (1028.32, "1028.32"), (1028.329, "1028.329"), (1028.3217, "1028.322"), (np.nan, "nan"), (np.inf, "inf"), ], ) def test_ftoa(f, a): f = np.array([f], dtype=np.float32)[0] buf = np.empty(FLOAT32_BUF_SIZE, dtype=np.uint8) p = ftoa(buf, 0, f) > assert byte_buf_to_str(buf[:p]) == a E AssertionError: assert '0' == '0.001' E E - 0.001 E + 0 sgkit/tests/io/vcf/test_vcf_writer_utils.py:96: AssertionError ========================================================================== short test summary info ========================================================================== FAILED sgkit/tests/io/vcf/test_vcf_writer_utils.py::test_ftoa[0.0005-0.001] - AssertionError: assert '0' == '0.001' ```

I think this was actually passing by accident before. NumPy will round towards the nearest even value, so in this case it would be 0.000, not 0.001. However, due to what I think were changes in NumPy around value-based promotion (https://numpy.org/neps/nep-0050-scalar-promotion.html) this test fails in NumPy 2, while it passed in NumPy 1. Also, in Numba it passes - when using NumPy 1 or 2, since Numba has not yet caught up on that aspect of NumPy 2 (i.e. it is still following NumPy 1 behaviour).

So I think it's best to drop the test for that case, since it is arguably incorrect anyway (0.0005 should round to 0).

tomwhite commented 2 months ago

I noticed that scikit-allel is now compatible with NumPy 2, which makes this a lot easier! (Thanks @alimanfoo for that!)

I have updated #1237.