Meson Build + Proper Windows Support

[jackm97]

Purpose

Managed to get a working meson build system going. With scipy making the switch recently, I managed to draw on their work quite a bit.

With meson out of the way I was able to get a Windows build working a lot easier and it only require altering one line of code in pyipoptcore.c. So, although this PR technically gets Windows working, it's more of a byproduct of the better build system hence combining them into one PR.

Expected time until merged

Not super urgent, but I am hoping to get a Windows build up on conda-forge sooner rather than later. I've managed to mess around with the conda-forge feedstock yml to get both Windows and Linux to build correctly. When this PR is in with a new version tag, I'll make a PR for the feedstock shortly after.

Type of change

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (non-backwards-compatible fix or feature)
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no API changes)
• Documentation update
• Maintenance update
• Other (please describe)

New build system. Changes a lot while also changing nothing at all

Testing

Linux passes all the tests. Windows however does not. IPOPT for test_rosenbrock fails with "92 != 0" and test_nsga2_multi_objective results in a heap corruption. For the time being I've added a platform check on both of these and warnings are issued for Windows so the tests still pass on Windows. I've ran the Windows build with my organization's software where we use IPOPT and PSQP quite heavily and all tests pass there. I'm hoping to get a conda-forge build out with a disclaimer for Windows until this is resolved.

The Windows tests also still need to be run in serial due to race conditions. We actually dealt with a similar problem in our software so I may be able to address this at another time.

Checklist

• I have run flake8 and black to make sure the code adheres to PEP-8 and is consistently formatted
• I have run unit and regression tests which pass locally with my changes
• I have added new tests that prove my fix is effective or that my feature works
• I have added necessary documentation

[codecov]

Codecov Report

Merging #300 (4f1b637) into main (4ae37e6) will decrease coverage by 11.69%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##             main     #300       +/-   ##
===========================================
- Coverage   84.17%   72.47%   -11.70%     
===========================================
  Files          22       22               
  Lines        3317     3317               
===========================================
- Hits         2792     2404      -388     
- Misses        525      913      +388     

Impacted Files	Coverage Δ
pyoptsparse/pyOpt_constraint.py	82.31% <ø> (ø)
pyoptsparse/pyOpt_objective.py	63.63% <ø> (ø)
pyoptsparse/pyOpt_variable.py	38.46% <ø> (ø)
pyoptsparse/__init__.py	100.00% <100.00%> (ø)
pyoptsparse/pySNOPT/pySNOPT.py	12.92% <0.00%> (-76.93%)	⬇️
pyoptsparse/pyNLPQLP/pyNLPQLP.py	25.68% <0.00%> (-66.98%)	⬇️
pyoptsparse/pyOpt_solution.py	57.77% <0.00%> (-42.23%)	⬇️
pyoptsparse/pyOpt_utils.py	60.46% <0.00%> (-7.91%)	⬇️
pyoptsparse/pyOpt_history.py	77.22% <0.00%> (-5.80%)	⬇️
pyoptsparse/pyOpt_optimizer.py	83.40% <0.00%> (-1.82%)	⬇️
... and 1 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[marcomangano]

Thanks for the effort Jack! I haven't tried the Meson build myself yet, but I have a few general comments below. The main issue is that changing the name of the nsga2 imported binary is breaking all the tests on the current Linux images.

How do you plan to maintain this build process? None of our codes run on Win so we will not generate Windows images in the foreseeable future. The only testing for this Meson build would happen when we release a new tag/version and the conda-forge repo is updated. Although I expect most of the short- and mid-term development to involve the python layer, any bugfix for Win would require some extra work on your side.

[ewu63]

Before I dive into the details of the PR, I wanted to discuss some high-level ideas. A few immediate questions:

• can this work with setuptools? I think I would prefer to let setuptools keep track of the package installation, but use meson to build the extension modules. This would be a two-step build process, where meson builds the .so files and copies them to the source directory, then pip installs the package (either in-source or in whichever environment the user wants, but this should be handled transparently). This is what we do with other packages from the lab (albeit using raw Makefiles). And then we can strip out all the numpy.distutils stuff from the various setup.py files.
• I'm a bit confused by the use of conda here, is it doing anything beyond providing a virtual environment in Windows? You're not using the cross-platform compilation capabilities of conda right?
• The compilation of optional optimizers (where we do not distribute the source code) should be detected automatically by meson, and extension modules built if available.

[jackm97]

@marcomangano

How do you plan to maintain this build process? None of our codes run on Win so we will not generate Windows images in the foreseeable future. The only testing for this Meson build would happen when we release a new tag/version and the conda-forge repo is updated. Although I expect most of the short- and mid-term development to involve the python layer, any bugfix for Win would require some extra work on your side.

Currently I'm working half to full time depending on the week, and I'm also a full-time grad student, so time is limited to say the least. That said pyoptsparse is essential for our code base at my job so I'm figuring out with them if I can maintain a Windows build in an official capacity. I expect this to not be a problem as we're a small team with not a lot of overhead. Worst-case, I do think I could maintain this in my free time but I'm wondering if we might be able to come up with a way of still moving releases forward without Windows in the case that I'm swamped.

The main issue is that changing the name of the nsga2 imported binary is breaking all the tests on the current Linux images.
Without numpy disutils I had to figure out the Swig command on my own and noticed the docs said this "When linking the module, the name of the output file has to match the name of the module prefixed by an underscore" after it failing to import without the underscore. However, this morning I found the relevant command-line option to bypass that. Will commit soon

[jackm97]

@nwu63

Before I dive into the details of the PR, I wanted to discuss some high-level ideas. A few immediate questions:

* can this work with setuptools? I think I would prefer to let setuptools keep track of the package installation, but use meson to build the extension modules. This would be a two-step build process, where meson builds the .so files and copies them to the source directory, then pip installs the package (either in-source or in whichever environment the user wants, but this should be handled transparently). This is what we do with other packages from the lab (albeit using raw Makefiles). And then we can strip out all the numpy.distutils stuff from the various `setup.py` files.

I mostly followed the scipy repo and these build instructions. I don't see why a setuptools should be a problem but I'm just not too familiar with setuptools. This is my first time working with Python build systems. I don't think scipy uses Meson yet when packaging but I could be mistaken. Setuptools is necessary for pip no? If so, it seems pretty important. If someone could point me in the right direction I could take a stab at it. I left in the legacy numpy distutils for now to maintain compatibility.

* I'm a bit confused by the use of conda here, is it doing anything beyond providing a virtual environment in Windows? You're not using the cross-platform compilation capabilities of conda right?

That's right. The build script itself doesn't assume a conda environment, just that all the dependency/executable paths are setup correctly so that meson can find them.

* The compilation of optional optimizers (where we do not distribute the source code) should be detected automatically by meson, and extension modules built if available.

I was thinking about how to best do this. I don't have these packages so I don't know their layout. Suggestions?

[jackm97]

I noticed black failed again after trying to fix the style check errors. It seems to be a line length problem. What's your line length?

[ewu63]

I mostly followed the scipy repo and these build instructions. I don't see why a setuptools should be a problem but I'm just not too familiar with setuptools. This is my first time working with Python build systems. I don't think scipy uses Meson yet when packaging but I could be mistaken. Setuptools is necessary for pip no? If so, it seems pretty important. If someone could point me in the right direction I could take a stab at it. I left in the legacy numpy distutils for now to maintain compatibility.

Right, yeah I think we can work together to figure this out. What I'm picturing is for meson to build the extension modules, and to move them into the source directories. Then setup.py will just install everything into whatever environment is set up.

We can even have an optional testing step or something via meson, but that's secondary I think.

That's right. The build script itself doesn't assume a conda environment, just that all the dependency/executable paths are setup correctly so that meson can find them.

Right I see, I think I would prefer documenting the build steps rather than committing the .bat, shell scripts, etc. into the repo.
The environment file should also be kept separate I think. Related to this, we do maintain a conda-forge package, but it doesn't support Windows yet.

I was thinking about how to best do this. I don't have these packages so I don't know their layout. Suggestions?

There's logic in the individual setup.py files to check this, so I can help you port that over into the meson files.

I noticed black failed again after trying to fix the style check errors. It seems to be a line length problem. What's your line length?

We use -l 120, see here for more info.

In terms of maintaining this, I think we can comfortably take over for Linux builds, and honestly this seems a lot more maintainable/understandable than the numpy.distutils stuff we had before. I don't think we will be testing Windows via our CI processes, so unless we can get the conda-forge package working with Windows, I'm afraid that will stay untested. Of course, if you use this regularly, then feel free to submit patches or whatever if anything breaks.

[jackm97]

So I have a coworker that runs osx and we're going back and forth trying to get macos to build. Is there any commit message I can use to avoid wasting your CI?

[jackm97]

@nwu63 I will reply more in depth to your message sometime next week. You can ignore my last message on running CI, I can just work on a different branch and rebase when it's all working haha. I'm a bit tired as of late, forgive me.

I've allowed edits by maintainers so feel free to add the setup.py if you want to get in before I can properly address your last comment. I agree that the meson build system is much more straight forward

[ewu63]

So I have a coworker that runs osx and we're going back and forth trying to get macos to build. Is there any commit message I can use to avoid wasting your CI?

Nah our CI is free and unlimited, don't worry about that.

@nwu63 I will reply more in depth to your message sometime next week. You can ignore my last message on running CI, I can just work on a different branch and rebase when it's all working haha. I'm a bit tired as of late, forgive me.

I've allowed edits by maintainers so feel free to add the setup.py if you want to get in before I can properly address your last comment. I agree that the meson build system is much more straight forward

Sure no worries, I'll push up some stuff this week and we can iterate.

[jackm97]

Sure no worries, I'll push up some stuff this week and we can iterate.

Great! Looking forward to a more painless build process across platforms. Btw I forked your conda feedstock for my conda-build testing and was making good progress. We may be building in-house soon to take advantage of snopt so we would put our Windows CI to work there and help catch bugs in tandem with the the conda-forge CI.

Another option I thought of is to maintain a separate conda-forge feedstock for windows, like pyoptsparse-win64 or something, for now. I imagine there will be a fair amount of trial and error there and I wouldn't want it to conflict with the progress of your feedstock

[eli-schwartz]

• can this work with setuptools? I think I would prefer to let setuptools keep track of the package installation, but use meson to build the extension modules. This would be a two-step build process, where meson builds the .so files and copies them to the source directory, then pip installs the package (either in-source or in whichever environment the user wants, but this should be handled transparently). This is what we do with other packages from the lab (albeit using raw Makefiles). And then we can strip out all the numpy.distutils stuff from the various setup.py files.

I mostly followed the scipy repo and these build instructions. I don't see why a setuptools should be a problem but I'm just not too familiar with setuptools. This is my first time working with Python build systems. I don't think scipy uses Meson yet when packaging but I could be mistaken. Setuptools is necessary for pip no? If so, it seems pretty important. If someone could point me in the right direction I could take a stab at it. I left in the legacy numpy distutils for now to maintain compatibility.

setuptools is not necessary for pip, pip expects one of the following:

• a PEP 517 build backend: https://peps.python.org/pep-0517/
• the implicit / default setuptools build backend

SciPy intends to use an alternative build backend which directly uses Meson and fully drop any and all references to setuptools, although currently during the transition period the default pip install experience uses setuptools and doesn't invoke Meson at all.

[ewu63]

@eli-schwartz I have a few questions regarding meson, apologies in advance since I'm not super familiar with it

• I'm still a bit confused about the interaction between meson and python packaging. The standard approach for installing in meson will just install it into the environment that's detected by meson right? When I tried this, pip was not aware of this installed package. Is there a way for pip to manage the installation (e.g uninstall using pip), get the version, etc.?
• More broadly speaking, I'm still more used to the setup.py approach where an in-source installation is allowed for development purposes (for example via pip install -e). Is there a way to replicate this behaviour? I guess an alternative is to somehow get meson to copy the .so files (which are really the only ones that need to be built by meson) to the source dir (where they currently live using numpy.distutils), but my suspicion is that this is not allowed in meson by design. How do most python packages get around this?

[jackm97]

@eli-schwartz I have a few questions regarding meson, apologies in advance since I'm not super familiar with it

• I'm still a bit confused about the interaction between meson and python packaging. The standard approach for installing in meson will just install it into the environment that's detected by meson right? When I tried this, pip was not aware of this installed package. Is there a way for pip to manage the installation (e.g uninstall using pip), get the version, etc.?
• More broadly speaking, I'm still more used to the setup.py approach where an in-source installation is allowed for development purposes (for example via pip install -e). Is there a way to replicate this behaviour? I guess an alternative is to somehow get meson to copy the .so files (which are really the only ones that need to be built by meson) to the source dir (where they currently live using numpy.distutils), but my suspicion is that this is not allowed in meson by design. How do most python packages get around this?

@nwu63 I know this wasn't directed at me but I think one option for maintaining backwards compatibility with setuptools is to first build and install the package into the current directory with meson at ./installdir or something. This would handle all the copying of .so files and such. We can add the setup.py files to the meson sources so they get copied over as well. Then we can run setup.py in the install directory for pip. This would unfortunately require a build step outside of the setup.py call which may not be okay. I'm not familiar with setuptools haha. I prefer conda-build as it supports local build and installs and has access to conda-forge which is just packed with software.

We could probably also run the meson build and install to $PWD/installdir from within setup.py but idk how that'd work with the moving of sources.

[ewu63]

Hey, so there are two main remaining TODOs (see this comment). I think this PR is ready so @sseraj and @marcomangano please review this PR and approve as you see fit. @jackm97 do you think you can figure out the conda build? Or do you think that's not necessary for this to get merged?

I guess we also want to bump the minor version? I can do that shortly.

[jackm97]

Hey, so there are two main remaining TODOs (see this comment). I think this PR is ready so @sseraj and @marcomangano please review this PR and approve as you see fit. @jackm97 do you think you can figure out the conda build? Or do you think that's not necessary for this to get merged?

I guess we also want to bump the minor version? I can do that shortly.

@nwu63 Yeah I can look at the conda-forge recipe. It should just be a matter of changing the source url. The build failure that happened was fixed I believe at some point but I'll get back into it and make sure. There are two approaches I see:

1. wait until the PR is merged into main, and then I can change to source url to main and version bump the conda-recipe
2. get it running now with the PR url, do a version bump, and then once it's merged to main change the url back to main.

Either way, the conda-forge PR should probably merged after the merge to main right?

[ewu63]

@nwu63 Yeah I can look at the conda-forge recipe. It should just be a matter of changing the source url. The build failure that happened was fixed I believe at some point but I'll get back into it and make sure. There are two approaches I see:

1. wait until the PR is merged into main, and then I can change to source url to main and version bump the conda-recipe

2. get it running now with the PR url, do a version bump, and then once it's merged to main change the url back to main.

Either way, the conda-forge PR should probably merged after the merge to main right?

Yeah this PR should be merged first, I was just wondering if we want to test this stuff in conda-forge before it's merged, in case we need to change something here for conda to build.

[sseraj]

I checked that the build works with Intel compilers on NAS. The new logging with subprocess is also nicer.
I'm not sure why one IPOPT test is failing on the Windows job though.

[jackm97]

Yeah this PR should be merged first, I was just wondering if we want to test this stuff in conda-forge before it's merged, in case we need to change something here for conda to build.

That makes sense. I'll get to it over the weekend or early next week! Looking forward to a Windows build on conda

[jackm97]

Hi all, some of the Windows tests failed on the conda-forge build. It's file permission errors which I find to be a common theme. On unix systems, as I'm sure you know, trying to remove a file that doesn't exist doesn't fail but it does on Windows. Could we add checks in the tests if files exist and are not being used by another process before read/write/delete? This is a problem within our own test framework as well.

I could do this but I'm still not that familiar with pyoptpsparse tests so if someone else would like to take it up I'll gladly pass the baton haha

But besides that, the builds on conda-forge all go to completion and tests succeed (sans minor Windows file errors and the disabled Windows tests)

[ewu63]

@jackm97 I've pushed up a fix, please try it out. If it works and we get the Azure job to pass, then we can safely merge this one (and then update the upstream on conda before merging that PR).

[jackm97]

@nwu63 All builds passed!

[ewu63]

LGTM! Merging 🎉