Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: build graph non-determinism #19167

Closed
mathstuf opened this issue Aug 31, 2023 · 6 comments · Fixed by #19168
Closed

BUG: build graph non-determinism #19167

mathstuf opened this issue Aug 31, 2023 · 6 comments · Fixed by #19168
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected
Milestone

Comments

@mathstuf
Copy link
Contributor

mathstuf commented Aug 31, 2023

Describe your issue.

It seems that some Cython code needs more dependencies specified. Namely, some Cython generation requires from scipy.special cimport cython_special to work. While trying to ninja -k0 in order to ninja -j1 to isolate the error message, the problem went away. This tells me that dependencies probably just need better ordered.

Reproducing Code Example

N/A

Error message

[674/845] Generating 'scipy/special/_ellip_harm_2.cpython-39-darwin.so.p/_ellip_harm_2.c'
FAILED: scipy/special/_ellip_harm_2.cpython-39-darwin.so.p/_ellip_harm_2.c
/Users/ben.boeckel/code/superbuild/build/install/bin/cython -3 --fast-fail --output-file scipy/special/_ellip_harm_2.cpython-39-darwin.so.p/_ellip_harm_2.c --include-dir . ../../scipy/special/_ellip_harm_2.pyx

Error compiling Cython file:
------------------------------------------------------------
...
from scipy.special cimport cython_special
^
------------------------------------------------------------

SciPy/NumPy/Python version and system information

SciPy 1.11.2
MesonPy 0.13.2
Meson 1.2.1
Cython 3.0.0
@mathstuf mathstuf added the defect A clear bug or issue that prevents SciPy from being installed or used as expected label Aug 31, 2023
@mathstuf
Copy link
Contributor Author

I suspect that adding cython_special as a dependency to the relevant py3.extension_module calls is needed…trying to figure out enough Meson to verify this hypothesis.

@rgommers
Copy link
Member

Thanks for the report. 0.13.2 is the meson-python version. Can you add the meson and Cython versions? This should be automatic, but it's very well possible that we're missing an explicit dependency here and that triggering the resulting problem is difficult.

@mathstuf
Copy link
Contributor Author

So I tracked it down to these two rules which should reproduce anywhere (may need a ninja -t clean first depending on how you get your builddir and hacking mesonpy (see Bartpab/meson-py#4) to get access to such a build directory):

% <generate meson build>
% ninja scipy/special/_ellip_harm_2.cpython-39-darwin.so.p/_ellip_harm_2.c
<fails as above>
% ninja scipy/special/cython_special.cpython-39-darwin.so
<works>
% ninja scipy/special/_ellip_harm_2.cpython-39-darwin.so.p/_ellip_harm_2.c
<now works>

@rgommers
Copy link
Member

Thanks! Yes, this is a clear bug. I've had on my wish/idea list for a long time to build every target separately to smoke out issues like these.

Here's how I reproduce it, from a clean repo:

$ meson setup build  # configure the build dir
$ # find an exact target name:
$ ninja -C build -t targets depth 0 | rg ".*ellip_harm.*.so"
$ # try to build only that target:
$ ninja -C build scipy/special/_ellip_harm_2.cpython-310-x86_64-linux-gnu.so
ninja: Entering directory `build'
[1/3] Generating 'scipy/special/_ellip_harm_2.cpython-310-x86_64-linux-gnu.so.p/_ellip_harm_2.c'.
FAILED: scipy/special/_ellip_harm_2.cpython-310-x86_64-linux-gnu.so.p/_ellip_harm_2.c 
/home/rgommers/mambaforge/envs/scipy-dev/bin/cython -3 --fast-fail --output-file scipy/special/_ellip_harm_2.cpython-310-x86_64-linux-gnu.so.p/_ellip_harm_2.c --include-dir . ../scipy/special/_ellip_harm_2.pyx

Error compiling Cython file:
------------------------------------------------------------
...
from scipy.special cimport cython_special
^
------------------------------------------------------------

/home/rgommers/code/scipy/scipy/special.pxd:1:0: 'scipy/special/cython_special.pxd' not found
ninja: build stopped: subcommand failed.

Meson is good at automatically adding dependencies, but it will miss the ones that come through a custom_target where we use a Python script to generate Cython source files which are then needed for a next target.

@mathstuf
Copy link
Contributor Author

I'm unsure of how many places need this added, but I suppose grepping pyx files for scipy imports would be a good guide.

@rgommers
Copy link
Member

I think it's only the places that use the output of a _generate_pyx.py script - that's easy to grep for.

mathstuf added a commit to mathstuf/scipy that referenced this issue Aug 31, 2023
Generating the source for `_ellip_harm_2` requires the `cython_special`
module to already be compiled. Add a dependency for this. Note that it
now must be a custom target because generators do not support extra
dependencies.

Fixes: scipy#19167
mathstuf added a commit to mathstuf/scipy that referenced this issue Aug 31, 2023
Generating the source for `_ellip_harm_2` requires the `cython_special`
module to already be compiled. Add a dependency for this. Note that it
now must be a custom target because generators do not support extra
dependencies.

Fixes: scipy#19167
rgommers added a commit to mathstuf/scipy that referenced this issue Sep 1, 2023
Generating the source for `_ellip_harm_2` requires the
`cython_special.pxd` header to be generated. So add a dependency for
this.

Closes scipygh-19167

Co-authored-by: Ralf Gommers <ralf.gommers@gmail.com>
rgommers added a commit that referenced this issue Sep 1, 2023
Generating the source for `_ellip_harm_2` requires the
`cython_special.pxd` header to be generated. So add a dependency for
this.

Closes gh-19167

Co-authored-by: Ralf Gommers <ralf.gommers@gmail.com>
@j-bowhay j-bowhay added this to the 1.12.0 milestone Sep 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants