MAINT: Add stand-alone libnpymath and remove dependency to numpy from npyrandom #25390

lysnikolaou · 2023-12-13T20:06:15Z

No description provided.

lysnikolaou · 2023-12-14T13:09:55Z

The test failures look like they come from test_cffi, but I don't really understand it. Does cffi have a problem parsing Py_ssize_t?

lysnikolaou · 2023-12-14T20:18:30Z

There's something weird going on on my system locally as well, which I don't really understand. The tests fails when ran with spin, but succeed when ran with pytest under the tools directory.

numpy on  remove-npyrandom-staticlib is 📦 v2.0.0.dev0 via 🐍 pyenv mambaforge (venv) 
❯ python -m spin test numpy/random/tests/test_extending.py -- -v
Invoking `build` prior to running tests:
$ /Users/lysnikolaou/repos/python/numpy/venv/bin/python vendored-meson/meson/meson.py compile -C build
INFO: autodetecting backend as ninja
INFO: calculating backend command to run: /Users/lysnikolaou/repos/python/numpy/venv/bin/ninja -C /Users/lysnikolaou/repos/python/numpy/build
ninja: Entering directory `/Users/lysnikolaou/repos/python/numpy/build'
[1/1] Generating numpy/generate-version with a custom command
Saving version to numpy/version.py
$ /Users/lysnikolaou/repos/python/numpy/venv/bin/python vendored-meson/meson/meson.py install --only-changed -C build --destdir ../build-install
$ export PYTHONPATH="/Users/lysnikolaou/repos/python/numpy/build-install/usr/lib/python3.11/site-packages"
$ /Users/lysnikolaou/repos/python/numpy/venv/bin/python -P -c 'import numpy'
$ export PYTHONPATH="/Users/lysnikolaou/repos/python/numpy/build-install/usr/lib/python3.11/site-packages"
$ cd /Users/lysnikolaou/repos/python/numpy/build-install/usr/lib/python3.11/site-packages
$ /Users/lysnikolaou/repos/python/numpy/venv/bin/python -m pytest --rootdir=/Users/lysnikolaou/repos/python/numpy/build-install/usr/lib/python3.11/site-packages -m 'not slow' numpy/random/tests/test_extending.py -v
========================================================== test session starts ===========================================================
platform darwin -- Python 3.11.3, pytest-7.4.0, pluggy-1.3.0 -- /Users/lysnikolaou/repos/python/numpy/venv/bin/python
cachedir: .pytest_cache
hypothesis profile 'np.test() profile' -> database=None, deadline=None, print_blob=True, derandomize=True, suppress_health_check=[HealthCheck.data_too_large, HealthCheck.filter_too_much, HealthCheck.too_slow, HealthCheck.large_base_example, HealthCheck.function_scoped_fixture]
rootdir: /Users/lysnikolaou/repos/python/numpy/build-install/usr/lib/python3.11/site-packages
configfile: ../../../../../pytest.ini
plugins: hypothesis-6.81.1, cov-4.1.0, xdist-3.3.1
collected 3 items / 1 deselected / 2 selected                                                                                            

numpy/random/tests/test_extending.py::test_numba SKIPPED (requires numba and cffi)                                                 [ 50%]
numpy/random/tests/test_extending.py::test_cffi FAILED                                                                             [100%]

================================================================ FAILURES ================================================================
_______________________________________________________________ test_cffi ________________________________________________________________

    @pytest.mark.skipif(cffi is None, reason="requires cffi")
    def test_cffi():
>       from numpy.random._examples.cffi import extending  # noqa: F401


numpy/random/tests/test_extending.py:118: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
numpy/random/_examples/cffi/extending.py:31: in <module>
    lib.random_standard_normal_fill(interface.bit_generator, n, vals_cffi)
        __builtins__ = <builtins>
        __cached__ = '/Users/lysnikolaou/repos/python/numpy/build-install/usr/lib/python3.11/site-packages/numpy/random/_examples/cffi/__pycache__/extending.cpython-311.pyc'
        __doc__    = '\nUse cffi to access any of the underlying C functions from distributions.h\n'
        __file__   = '/Users/lysnikolaou/repos/python/numpy/build-install/usr/lib/python3.11/site-packages/numpy/random/_examples/cffi/extending.py'
        __loader__ = <_frozen_importlib_external.SourceFileLoader object at 0x1079c9c30>
        __name__   = 'numpy.random._examples.cffi.extending'
        __package__ = 'numpy.random._examples.cffi'
        __spec__   = ModuleSpec(name='numpy.random._examples.cffi.extending', loader=<_frozen_importlib_external.SourceFileLoader object at...ysnikolaou/repos/python/numpy/build-install/usr/lib/python3.11/site-packages/numpy/random/_examples/cffi/extending.py')
        bit_gen    = <numpy.random._pcg64.PCG64 object at 0x10643b520>
        cffi       = <module 'cffi' from '/Users/lysnikolaou/repos/python/numpy/venv/lib/python3.11/site-packages/cffi/__init__.py'>
        ffi        = <cffi.api.FFI object at 0x1079c95f0>
        inc_dir    = '/Users/lysnikolaou/repos/python/numpy/build-install/usr/lib/python3.11/site-packages/numpy/_core/include/numpy'
        interface  = interface(state_address=4400067976, state=<cdata 'void *' 0x10643b588>, next_uint64=<cdata 'uint64_t(*)(void *)' 0x107...void *)' 0x107963660>, next_double=<cdata 'double(*)(void *)' 0x107963690>, bit_generator=<cdata 'void *' 0x10643b540>)
        lib        = <cffi.api._make_ffi_library.<locals>.FFILibrary object at 0x105d03490>
        n          = 100
        np         = <module 'numpy' from '/Users/lysnikolaou/repos/python/numpy/build-install/usr/lib/python3.11/site-packages/numpy/__init__.py'>
        os         = <module 'os' from '/Users/lysnikolaou/repos/python/cpython-versions/3.11.3/build/lib/python3.11/os.py'>
        parse_distributions_h = <function parse_distributions_h at 0x1079a6db0>
        rng        = Generator(PCG64) at 0x1079EBD50
        state      = {'bit_generator': 'PCG64', 'has_uint32': 0, 'state': {'inc': 14220707047729487342796260036649536101, 'state': 231538475895373809649456743026176147886}, 'uinteger': 0}
        vals_cffi  = <cdata 'double[100]' owning 800 bytes>
../../../../../venv/lib/python3.11/site-packages/cffi/api.py:912: in __getattr__
    make_accessor(name)
        make_accessor = <function _make_ffi_library.<locals>.make_accessor at 0x107d31f40>
        name       = 'random_standard_normal_fill'
        self       = <cffi.api._make_ffi_library.<locals>.FFILibrary object at 0x105d03490>
../../../../../venv/lib/python3.11/site-packages/cffi/api.py:908: in make_accessor
    accessors[name](name)
        FFILibrary = <class 'cffi.api._make_ffi_library.<locals>.FFILibrary'>
        accessors  = {'random_beta': <function _make_ffi_library.<locals>.accessor_function at 0x107d31860>, 'random_binomial': <function _...0x107d31860>, 'random_binomial_inversion': <function _make_ffi_library.<locals>.accessor_function at 0x107d31860>, ...}
        ffi        = <cffi.api.FFI object at 0x1079c95f0>
        library    = <cffi.api._make_ffi_library.<locals>.FFILibrary object at 0x105d03490>
        name       = 'random_standard_normal_fill'
        update_accessors = <function _make_ffi_library.<locals>.update_accessors at 0x107d31e90>
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

name = 'random_standard_normal_fill'

    def accessor_function(name):
        key = 'function ' + name
        tp, _ = ffi._parser._declarations[key]
        BType = ffi._get_cached_btype(tp)
>       value = backendlib.load_function(BType, name)
E       AttributeError: function/symbol 'random_standard_normal_fill' not found in library '/Users/lysnikolaou/repos/python/numpy/build-install/usr/lib/python3.11/site-packages/numpy/random/_generator.cpython-311d-darwin.so': dlsym(0x80452540, random_standard_normal_fill): symbol not found

BType      = <ctype 'void(*)(bitgen_t *, intptr_t, double *)'>
_          = 0
backendlib = <clibrary '/Users/lysnikolaou/repos/python/numpy/build-install/usr/lib/python3.11/site-packages/numpy/random/_generator.cpython-311d-darwin.so'>
ffi        = <cffi.api.FFI object at 0x1079c95f0>
key        = 'function random_standard_normal_fill'
library    = <cffi.api._make_ffi_library.<locals>.FFILibrary object at 0x105d03490>
name       = 'random_standard_normal_fill'
tp         = <void(*)(bitgen_t *, intptr_t, double *)>

../../../../../venv/lib/python3.11/site-packages/cffi/api.py:838: AttributeError
======================================================== short test summary info =========================================================
FAILED numpy/random/tests/test_extending.py::test_cffi - AttributeError: function/symbol 'random_standard_normal_fill' not found in library '/Users/lysnikolaou/repos/python/numpy/build-insta...
=============================================== 1 failed, 1 skipped, 1 deselected in 2.23s ===============================================

numpy on  remove-npyrandom-staticlib is 📦 v2.0.0.dev0 via 🐍 pyenv mambaforge (venv) took 11s 
❯ cd tools                                                      

numpy/tools on  remove-npyrandom-staticlib via 🐍 pyenv mambaforge (venv) 
❯ pytest --pyargs numpy.random.tests.test_extending -v          
========================================================== test session starts ===========================================================
platform darwin -- Python 3.11.3, pytest-7.4.0, pluggy-1.3.0 -- /Users/lysnikolaou/repos/python/numpy/venv/bin/python
cachedir: .pytest_cache
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase('/Users/lysnikolaou/repos/python/numpy/tools/.hypothesis/examples')
rootdir: /Users/lysnikolaou/repos/python/numpy
configfile: pytest.ini
plugins: hypothesis-6.81.1, cov-4.1.0, xdist-3.3.1
collected 3 items                                                                                                                        

../venv/lib/python3.11/site-packages/numpy/random/tests/test_extending.py::test_cython PASSED                                      [ 33%]
../venv/lib/python3.11/site-packages/numpy/random/tests/test_extending.py::test_numba SKIPPED (requires numba and cffi)            [ 66%]
../venv/lib/python3.11/site-packages/numpy/random/tests/test_extending.py::test_cffi PASSED                                        [100%]

===================================================== 2 passed, 1 skipped in 18.36s ======================================================

rgommers · 2023-12-14T20:54:54Z

This does look promising. It looks like there's still a bit of polishing to do, but it looks about right. Changing this code to regular C99-compliant code should make it more reusable.

ngoldbaum · 2023-12-20T16:39:40Z

Just did a quick glance - what about the public half float API? Don't we need to add it to the NumPy C API as well?

lysnikolaou · 2023-12-20T17:16:04Z

Don't we need to add it to the NumPy C API as well?

Do you mean move it out of npymath and add it to numpy?

ngoldbaum · 2023-12-20T17:20:27Z

Yes, otherwise downstream consumers of the half float API will need to add a new compile-time dependency. All the other stuff in npymath in principle they could migrate away from, as scipy has, but there's no other API for working with that dtype, and I think it belongs in NumPy with the rest of C API for working with numpy dtypes.

ngoldbaum · 2023-12-20T18:21:04Z

Yes, otherwise downstream consumers of the half float API will need to add a new compile-time dependency. All the other stuff in npymath in principle they could migrate away from, as scipy has, but there's no other API for working with that dtype, and I think it belongs in NumPy with the rest of C API for working with numpy dtypes.

We chatted about this at the community meeting and I withdraw this.

IEEE Half float is more widely supported in compilers than I realized, there's no need for downstream to have an API available in numpy. They can either use the support built in to their compiler or vendor the static library.

rgommers · 2024-01-12T17:56:52Z

The cffi failure is due to symbol visibility; in random/_generator.so the random_standard_normal_fill symbol and others like it are private (t output from nm) while when _generator.so is built by linking in the separate libnpymath.a they're public (T output from nm).

However, it looks to me like the test and the docs at https://numpy.org/devdocs/reference/random/examples/cffi.html are wrong. No one has any business digging into the symbols of _generator - nor should they even expect that private extension module to exist. The second piece of documentation mentions _generator too, but doesn't actually use it in the code: https://numpy.org/devdocs/reference/random/extending.html#cffi.

I think the right change to make is to change the problematic example/test, avoiding any mention of _generator, and make it use rng.bit_generator.cffi which is public. Probably in a separate PR that can be merged quickly, and then rebase this PR on top.

@mattip do you agree?

charris · 2024-02-06T15:09:58Z

This might need a release note, although it looks like this may not make it into 2.0.0.

lysnikolaou · 2024-02-06T15:25:36Z

I've been pushing changes to the libnpymath repo and this PR with fixes for the f2py issues and s390x. After the libnpymath PRs get merged, the only outstanding issue would be the one related to test_cffi. I'm trying to fix the test there, according to @rgommers's suggestion, but I know too little about cffi for it to go much faster.

I'll certainly add a release note, as soon as CI passes.

lysnikolaou · 2024-02-06T17:18:27Z

@rgommers @mattip Regarding the test_cffi issue (which is the only one that remains), I'm not sure how exactly that test can be fixed. Would it make sense to follow the approach mentioned in the numba + cffi example. Otherwise I can't see how we get access to those symbols with just rng.bit_generator.cffi.

rgommers · 2024-02-15T13:06:46Z

Would it make sense to follow the approach mentioned in the numba + cffi example. Otherwise I can't see how we get access to those symbols with just rng.bit_generator.cffi.

I think that that example is consistent with the approach here, since what it does is simply tell the user to vendor distributions.c and use symbols from there. There is no use of any private numpy libraries or extension modules, only rng.bit_generator.cffi. I think that is the only reasonable thing to do. Please feel free to remove or modify any test or example that makes use of anything that starts with an underscore.

To not bloat the size of this PR further, it's also fine with me to mark such tests as xfail or delete them, and file follow-up issues.

MAINT: Remove npy dependency from npyrandom and ship sources

9b6d920

github-actions bot added the 03 - Maintenance label Dec 13, 2023

lysnikolaou mentioned this pull request Dec 13, 2023

MAINT: Vendor npymath as git submodule and vendor npyrandom directly scipy/scipy#19638

Open

Fix types

c69465f

lysnikolaou force-pushed the remove-npyrandom-staticlib branch from 69f04a6 to c69465f Compare December 14, 2023 16:36

Fix typedefs

e764f44

rgommers mentioned this pull request Dec 15, 2023

ENH: Future of npymath library #20880

Open

lysnikolaou added 2 commits December 18, 2023 16:46

Merge branch 'main' into remove-npyrandom-staticlib

b9d83e8

Use stand-alone libnpymath and remove npymath from numpy source

731fc5b

lysnikolaou changed the title ~~MAINT: Remove npy dependency from npyrandom and ship sources~~ MAINT: Add stand-alone libnpymath and remove dependency to numpy from npyrandom Dec 19, 2023

lysnikolaou force-pushed the remove-npyrandom-staticlib branch 7 times, most recently from 2f8b749 to 0a82224 Compare December 22, 2023 12:40

Update libnpymath to fix test failures

04965fe

lysnikolaou force-pushed the remove-npyrandom-staticlib branch from 0a82224 to 04965fe Compare December 22, 2023 13:08

lysnikolaou added 2 commits January 11, 2024 16:22

Remove lysnikolaou/libnpymath submodule

2f8f8a1

Add numpy/libnpymath submodule

0c504ce

Merge branch 'main' into remove-npyrandom-staticlib

e4fe09b

lysnikolaou added 5 commits January 19, 2024 14:09

Update libnpymath to fix have_features detection

882b6b1

Merge branch 'main' into remove-npyrandom-staticlib

2b5da14

Add libnpymath dep in fft module

d89bd07

Update libnpymath to get fixes for FreeBSD

511c1cc

Add libnpymath dependency to f2py's meson.build

5b2cc62

Change npy_creal/npy_cimag to get pointers

91055f2

lysnikolaou added 2 commits February 9, 2024 10:33

Pull libnpymath changes & change includes / calls to new symbols

2284c17

Merge branch 'main' into remove-npyrandom-staticlib

c962c6c

lysnikolaou added 3 commits February 27, 2024 13:04

Merge branch 'main' into remove-npyrandom-staticlib

e90dcc7

Mark test_cffi as xfail

0078aaa

Update to use new version of libnpymath

dec8bac

lysnikolaou force-pushed the remove-npyrandom-staticlib branch from 433a796 to dec8bac Compare February 27, 2024 17:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MAINT: Add stand-alone libnpymath and remove dependency to numpy from npyrandom #25390

MAINT: Add stand-alone libnpymath and remove dependency to numpy from npyrandom #25390

lysnikolaou commented Dec 13, 2023

lysnikolaou commented Dec 14, 2023

lysnikolaou commented Dec 14, 2023

rgommers commented Dec 14, 2023

ngoldbaum commented Dec 20, 2023

lysnikolaou commented Dec 20, 2023 •

edited

Loading

ngoldbaum commented Dec 20, 2023

ngoldbaum commented Dec 20, 2023

rgommers commented Jan 12, 2024

charris commented Feb 6, 2024

lysnikolaou commented Feb 6, 2024

lysnikolaou commented Feb 6, 2024 •

edited

Loading

rgommers commented Feb 15, 2024

MAINT: Add stand-alone libnpymath and remove dependency to numpy from npyrandom #25390

Are you sure you want to change the base?

MAINT: Add stand-alone libnpymath and remove dependency to numpy from npyrandom #25390

Conversation

lysnikolaou commented Dec 13, 2023

lysnikolaou commented Dec 14, 2023

lysnikolaou commented Dec 14, 2023

rgommers commented Dec 14, 2023

ngoldbaum commented Dec 20, 2023

lysnikolaou commented Dec 20, 2023 • edited Loading

ngoldbaum commented Dec 20, 2023

ngoldbaum commented Dec 20, 2023

rgommers commented Jan 12, 2024

charris commented Feb 6, 2024

lysnikolaou commented Feb 6, 2024

lysnikolaou commented Feb 6, 2024 • edited Loading

rgommers commented Feb 15, 2024

lysnikolaou commented Dec 20, 2023 •

edited

Loading

lysnikolaou commented Feb 6, 2024 •

edited

Loading