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

Problems switching from numpy.distutils to meson #10536

Closed
aburrell opened this issue Jun 24, 2022 · 15 comments
Closed

Problems switching from numpy.distutils to meson #10536

aburrell opened this issue Jun 24, 2022 · 15 comments

Comments

@aburrell
Copy link

Describe the bug
This is likely not a bug, but rather a lack of understanding on the correct procedures. I've been working on this on and off for a bit, and am now stuck and hoping for a bit of help or a shove in a useful direction.

My problem is that I can't get meson to build my python-wrapped-legacy-fortran on Windows. This works on my local computer (OS X Big Sur). My access to Windows is only through GitHub Actions, and the latest failure can be seen here: https://github.com/aburrell/apexpy/runs/7048848662?check_suite_focus=true

It looks like the compiler isn't finding the Python.h header. This header exists and meson.build can find it (see the meson build step). I am out of ideas on how to fix this. I also tried using miniconda, but that failed much earlier (in the build step, due to the binutils library version not being new enough to be used with meson and no newer library available).

To Reproduce
You can check out the working branch at the public repository: https://github.com/aburrell/apexpy/tree/reorg_w_meson

Expected behavior
Meson being able to build and install the code on Windows.

system parameters

  • Is this a cross build or just a plain native build (for the same computer)? native build
  • what operating system (e.g. MacOS Catalina, Windows 10, CentOS 8.0, Ubuntu 18.04, etc.): Microsoft Windows Server 2022
  • what Python version are you using e.g. 3.8.0: Python 3.7.9
  • what meson --version: meson 0.62.2
  • what ninja --version if it's a Ninja build: ninja 1.11.0
@eli-schwartz
Copy link
Member

It looks like it compiles fine, nothing wrong with Python.h -- but linking the module is having issues because PyFortran_Type isn't available.

The interesting thing is that numpy.distutils will automatically call f2py for you, and that generates some stuff in e.g. build/src.linux-x86_64-3.10/ -- this includes build/src.linux-x86_64-3.10/build/src.linux-x86_64-3.10/fortranapex/fortranobject.c in which that type is defined.

This doesn't seem to be getting done in your meson.build. @rgommers probably knows the most about this, but looking at the SciPy build, specifically https://github.com/scipy/scipy/blob/55fca818b49d5730c3cc77d012a593a910f3c03c/scipy/meson.build#L52
It is grabbing fortranobject.c from inside numpy.f2py's installed data directory src/, and adding it to various extension modules all over the tree.

@eli-schwartz
Copy link
Member

It's entirely possible that in the long run, Meson will handle this in some more integrated fashion. e.g. adding a numpy and/or f2py dependency that wires up the numpy get_includes() directory, or adds fortranobject as part of the f2py interface, or follow in the footsteps of our cython support and run f2py on .pyf files. It likely depends on what NumPy/SciPy communicate with us that they'd like to see -- we're still mutually exploring how to make things better now that the initial SciPy migration is completed using custom coded handling.

@eli-schwartz
Copy link
Member

The install declarations aren't quite correct yet either. Here's a patch that demonstrates how to do it correctly:

From 9a089fe2d89a6dd4f640e6f760e2c43e0631780f Mon Sep 17 00:00:00 2001
From: Eli Schwartz <[email protected]>
Date: Fri, 24 Jun 2022 18:39:31 -0400
Subject: [PATCH] meson: fully install all files

- extension modules importable as "foo.bar" must be named "bar" and
  installed via `subdir: 'foo'`; naming them "foo.bar" will attempt to
  install a top-level file named "foo.bar.$EXT" which python does not
  know how to import
- install all the pure python sources
---
 meson.build | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index b900624..8d4f86d 100644
--- a/meson.build
+++ b/meson.build
@@ -119,11 +119,25 @@ endif
 # Check the python headers
 cc.check_header('Python.h', dependencies: [py3_dep], required: true)
 
-py3.extension_module('apexpy.fortranapex',
+py3.extension_module('fortranapex',
   'fortranapex/magfld.f', 'fortranapex/apex.f', 'fortranapex/makeapexsh.f90',
   'fortranapex/igrf.f90', 'fortranapex/apexsh.f90',
   'fortranapex/checkapexsh.f90', 'fortranapex/fortranapexmodule.c',
   include_directories: inc_dirs,
   c_args: numpy_nodepr_api,
   dependencies : py3_dep,
+  subdir: 'apexpy',
   install : true)
+
+py3.install_sources(
+  'apexpy/apex.py',
+  'apexpy/apexsh.dat',
+  'apexpy/_copyfiles.py',
+  'apexpy/_gcc_build_bitness.py',
+  'apexpy/helpers.py',
+  'apexpy/igrf13coeffs.txt',
+  'apexpy/__init__.py',
+  'apexpy/__main__.py',
+  pure: false,
+  subdir: 'apexpy'
+)
-- 
2.35.1

@rgommers
Copy link
Contributor

It is grabbing fortranobject.c from inside numpy.f2py's installed data directory src/, and adding it to various extension modules all over the tree.

Yep, I think that's the simplest way for now. I recommend copying https://github.com/scipy/scipy/blob/55fca818b49d5730c3cc77d012a593a910f3c03c/scipy/meson.build#L39-L52, and after that every extension module that uses f2py should have dependencies like https://github.com/scipy/scipy/blob/55fca818b49d5730c3cc77d012a593a910f3c03c/scipy/interpolate/meson.build#L142-L146

It's entirely possible that in the long run, Meson will handle this in some more integrated fashion. e.g. adding a numpy and/or f2py dependency

Yes, we should do that in some form or another. This is tracked in gh-9598.

@eli-schwartz
Copy link
Member

Hmm, but do you actually need to recompile fortranobject.c for each extension I wonder.

@rgommers
Copy link
Contributor

Not necessarily; that's how numpy.distutils did it, but you could build it as a static library and then link it in I think. Not sure it gains much compilation time wise, but it could avoid duplicate warnings for example.

@eli-schwartz
Copy link
Member

eli-schwartz commented Jun 26, 2022

I'm looking at a local patch for this, it reduces the number of build steps by 22 (down to 1551, which isn't all that much in proportion).

EDIT: and there's a 9-line warning it produces.

@rgommers
Copy link
Contributor

@eli-schwartz thanks! If you want to submit a PR or point me to your branch so I can test it, that'd be great.

@eli-schwartz
Copy link
Member

scipy/scipy#16475

@rgommers
Copy link
Contributor

Ah, didn't see that (I'm not watching the whole repo) - thanks!

@aburrell
Copy link
Author

aburrell commented Sep 6, 2022

Hi, thanks for the help so far! I'm still stuck at (basically) the same place. I have updated things by:

  • Fixing the bugs found by @eli-schwartz
  • Copying the code blocks highlighted by @rgommers
  • Updating the pre-F77 fortran to F90 standards (in case that would be a problem).

These changes can be found here: aburrell/apexpy#93. I am (at least) getting the similar errors locally on Mac as I am on the Windows CI. It does install, but not correctly, with the following error:

 dlopen(/Users/aburrell/Programs/Git/apexpy/apexpy/fortranapex.cpython-39-darwin.so, 2): Symbol not found: _f2pywrapfint_

I am sure I am not linking the F2PY library correctly, but I can't see what else I can change.

@rgommers
Copy link
Contributor

rgommers commented Sep 6, 2022

@aburrell it's likely that the compiler toolchains you are using matters here. What are you using for C/C++ and Fortran compilers on each platform?

I had a look at the failing CI jobs, and they are still using numpy.distutils. You need to update the install command in your PR from python setup.py to pip install . or similar.

@aburrell
Copy link
Author

aburrell commented Sep 6, 2022

@rgommers Thanks for your help. Changing to use pip has an odd result: on Windows it uses meson, but on mac/linux it still calls setuptools. I haven't been able to figure out how to change that. I don't see why it would happen, since I specified 'mesonpy' as the build backend in the pyproject.toml.

@rgommers
Copy link
Contributor

rgommers commented Sep 7, 2022

@aburrell you changed to pip install -e ., which isn't supported yet by meson-python (it's in the works): https://github.com/FFY00/meson-python/issues/47. You want simply pip install ..

It would probably be easier if you ask me any follow-up questions on your PR, given that that's where the code is and there's no Meson bug here. How about we close this, and you can at-mention me in case you need more help?

@aburrell
Copy link
Author

aburrell commented Sep 7, 2022

Closing this issue as it's not a bug in meson, just me not being able to figure it out without help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants