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

Tracking issue for Meson support of submodules #22

Closed
31 of 41 tasks
rgommers opened this issue Jun 19, 2021 · 31 comments · Fixed by numpy/numpy#21187
Closed
31 of 41 tasks

Tracking issue for Meson support of submodules #22

rgommers opened this issue Jun 19, 2021 · 31 comments · Fixed by numpy/numpy#21187

Comments

@rgommers
Copy link
Owner

rgommers commented Jun 19, 2021

Here is a list to keep track of where we are with adding Meson support. An "x" means "has a meson.build file that's mostly complete; some TODO or FIXME items may be left").

  • _lib
  • cluster
  • constants
  • fft
  • fftpack
  • integrate
  • interpolate
  • io
  • linalg
  • misc
  • ndimage
  • odr
  • optimize
  • signal
  • sparse
  • sparse.linalg
  • sparse.csgraph
  • spatial
  • special
  • stats

When picking up a new module, let's comment here to avoid double work.

Other topics worth tracking separately:

@czgdp1807
Copy link

I will be working on adding stats module. Thanks.

@czgdp1807
Copy link

I will pick up spatial module next. Many modules seem to depend on it.

@rgommers
Copy link
Owner Author

Indeed. Sounds good. I will have a go at special.

@czgdp1807
Copy link

I will work on sparse.linalg. Refer - #28 (comment)

@rgommers
Copy link
Owner Author

We have a working CI job now.

@czgdp1807
Copy link

Note - I can keep working on adding modules along with other things as that part happens quickly (mostly everything is setup already, just meson.build files are required to be added). Though, I will not be able to work on infrastructure part like, testing and fixing things on different architectures and OSes. Thanks.

@czgdp1807
Copy link

Next module - interpolate. Let me know if it works.

@rgommers
Copy link
Owner Author

rgommers commented Jul 2, 2021

Sounds great, thanks @czgdp1807!

@rgommers
Copy link
Owner Author

rgommers commented Jul 3, 2021

@Smit-create do you want to add Meson support to misc? It should be straightforward, the purpose of doing this is to get familiar with the Meson build system and building SciPy.

To start, I suggest:

  • read https://github.com/rgommers/scipy/blob/meson/MESON_BUILD.md and the first parts of the Meson docs
  • look at another submodule for an example, e.g. scipy/fft/meson.build
  • once it works, enable tests for the module at the bottom of mesondev.sh
  • submit a PR to the meson integration branch on this repo, there is one CI job that should pass

@Smit-create
Copy link

Yes, I will refer to the docs and try to take it up. Thanks!

@rgommers
Copy link
Owner Author

rgommers commented Jul 6, 2021

misc done. @Smit-create do you want to do signal next? That has some C and Cython code, so should be a little more nontrivial:) There's one .in file, that is a Tempita template. There's a couple of examples of how to deal with those, for example at the top of scipy/sparse/meson.build

@Smit-create
Copy link

do you want to do signal next?

Yes, will pick that up next.

There's a couple of examples of how to deal with those

Thanks, I will look at them.

@czgdp1807
Copy link

Hi. Once I complete interpolate then I will be taking up io. Just dropping in here for information. Thanks.

@rgommers rgommers mentioned this issue Jul 6, 2021
19 tasks
@rgommers
Copy link
Owner Author

rgommers commented Jul 8, 2021

All submodules complete! I'm going to have a go at squashing the last Cython related dependency issues, and then (fingers crossed) the whole test suite will run.

@rgommers
Copy link
Owner Author

So far this tracking issue focused on what we need specifically for SciPy. There's a number of related things that are in flight, so let me add a summary of what and who here:

@ev-br
Copy link

ev-br commented Sep 16, 2021

Implementing depfile support in Cython (cython / c++ meson upstream tracking issue ev-br/mc_lib#36, Add support for GCC style dependency file cython/cython#1459) (@ev-br)

I think I've an idea of where to start with this, the basics are in the commit message referenced in the issue. No idea of how this will go with cython devs though. And I ran out of time with this before the start of the teaching term, will come back to it at some point unless someone beats me to it.

@rgommers
Copy link
Owner Author

rgommers commented Oct 2, 2021

And I ran out of time with this before the start of the teaching term, will come back to it at some point unless someone beats me to it.

That sounds like a ways away. Reading ev-br/mc_lib@818f466, it may not be that hard. @ev-br do you think this is feasible for an intern new to Cython & build stuff to tackle?

@ev-br
Copy link

ev-br commented Oct 2, 2021

do you think this is feasible for an intern new to Cython & build stuff to tackle?

Yup, this is very much doable.

The main challenges seem to be non-technical : (1) plumb this through to the cythonize cmd utility, (2) convince meson people to call it instead of the cython executable :-).

@eli-schwartz
Copy link

Why can't it be plumbed through the cython utility (which has a simpler and more straightforward interface)?

See my comment (and surrounding discussion) at mesonbuild/meson#8706 (comment)

cythonize by default tries to guess what you want, and is geared to building packages itself. It is notably missing its own option for --include-dir and --output-file...

It could possibly be made to work, but tbh I'd expect a cython implementation of depfile support should, for the sake of thoroughness, add that option to both executables -- and this should hopefully not be harder than adding it to just one.

@rgommers
Copy link
Owner Author

rgommers commented Oct 3, 2021

convince meson people to call it instead of the cython executable :-).

I agree with @eli-schwartz, that seems like a nonstarter. cython does exactly what we want, cythonize does way too much and uses setuptools.

@ev-br
Copy link

ev-br commented Oct 3, 2021

I agree with @eli-schwartz, that seems like a nonstarter. cython does exactly what we want, cythonize does way too much and uses setuptools.

There seem to be at least two differing opinons on this :-). cython/cython#1214 (comment)

@eli-schwartz
Copy link

It's not clear to me that the concerns raised there are relevant to emitting dependency rules for rerunning cython in order to transpile .pxd into .cpp

@rgommers
Copy link
Owner Author

rgommers commented Oct 3, 2021

I indeed don't think those concerns are relevant. cython can do .pyx -> .c/.cpp, which means it must have all the necessary info for doing that available internally somewhere. And those generated files are guaranteed to be portable as well (we used to ship them in sdists), so there's nothing regarding C/C++ compilers that Cython can possibly care about.

@rgommers
Copy link
Owner Author

rgommers commented Oct 3, 2021

I have re-read the Cython docs, and this bit at the top of the "source files and compilation" page isn't quite complete:

There are two ways of compiling from the command line.

  • The cython command takes a .py or .pyx file and compiles it into a C/C++ file.
  • The cythonize command takes a .py or .pyx file and compiles it into a C/C++ file. It then compiles the C/C++ file into an extension module which is directly importable from Python.

That would mean that cythonize invokes a C/C++ compiler, which would be mixing two build systems - and that's a recipe for disaster.

However, that is not what cythonize actually does. Instead, it returns a set of distutils.Extension objects. Those could be discarded by Meson after inspecting them, rather than running them with setuptools. But that would tie us back to distutils/setuptools, which we have a strong desire to avoid. So it's still quite clear that cythonize is the wrong thing, and cython the right thing, to use.

@eli-schwartz
Copy link

IIRC the cythonize command can be told to not invoke a C++ compiler, in which case it acts as though you invoked cython (but with a less powerful/usable set of command line options) and the author agrees that there's no real point in using the former.

Using the cythonize() function as a python module has a function return value of distutils/setuptools Extension objects. This is not on the table for discussion, meson is not going to import cython as a module and use its function API. So whether it uses distutils/setuptools is not important.

@swallan
Copy link

swallan commented Jan 8, 2022

Hello Ralph :)

I followed your guide to setup SciPy with meson on Mac. One thing that I noticed that might need to be adjusted (or noted in the guide) is to make sure that the python version in this path is correct from this instruction:
Screen Shot 2022-01-07 at 3 57 56 PM

I wasn't paying the closest attention, but my actual path is scipy/installdir/lib/python3.10/site-packages/, so 3.10 not 3.9. It's a easy fix, but I thought I would mention that this happened when I was just following the guide as one would normally! (I have since fixed the version to 3.9)

@rgommers
Copy link
Owner Author

rgommers commented Jan 8, 2022

Thanks @swallan. I will add a code comment to note this in the next update for meson.build files.

@rgommers
Copy link
Owner Author

rgommers commented Jan 30, 2022

I had a look at where we are at and what are actual release blockers for 1.9.0. These are:

  1. Windows build must pass (currently segfaulting on HiGHS)
  2. Meson virtualenv handling improvements (PR ready and marked for 0.62.0)
  3. Support for MKL and other BLAS/LAPACK libraries in Meson (we need to work on this, should go into 0.62.0 which based on current release cadence should land in April)
  4. Wheel building on all supported platforms
  5. CI jobs for all relevant configs ported to Meson in the SciPy main branch
  6. sdist correctness needs to be verified - are we installing .pxd headers etc.?

Everything else is nice to have:

  • Cython depfile support in Meson is very useful for local development but doesn't affect a release
  • A number of things needed in Meson to reduce the number of warnings emitted and simplify our meson.build files - all improvements and important for adoption by other Python projects, but not blocking
  • A cleanup of all the SciPy docs on building from source would be great, but they're always outdated anyway so also not blocking for a release
  • ILP64 support is probably highest on the list of nice-to-haves and we should aim to get that implemented in time for 1.9.0, but not important enough that it's a hard blocker (a setup.py based build will still be usable for 1.9.0)

@eli-schwartz
Copy link

Meson virtualenv handling improvements (PR ready and marked for 0.62.0)

PR now merged to meson git master in the middle of the 0.62.0 development cycle.

@rgommers
Copy link
Owner Author

Nice, we're progressing nicely: we're down to sdist/wheel builds (almost there, some macOS and Windows testing left to do), test/improve MKL support, and some CI job porting as release blockers.

@rgommers
Copy link
Owner Author

I opened scipy#16293 for remaining items. I think we're good here, this issue served its purpose, so I'll close it. Thanks everyone.

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

Successfully merging a pull request may close this issue.

6 participants