pyoptsparse icon indicating copy to clipboard operation
pyoptsparse copied to clipboard

Build with mesonpy

Open ewu63 opened this issue 10 months ago • 20 comments

Purpose

Continuation of #431 but from the mdolab branch to test all optimizers.

Expected time until merged

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)

Testing

Checklist

  • [ ] I have run flake8 and black to make sure the Python code adheres to PEP-8 and is consistently formatted
  • [ ] I have formatted the Fortran code with fprettify or C/C++ code with clang-format as applicable
  • [ ] 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

ewu63 avatar Apr 09 '25 01:04 ewu63

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 86.70%. Comparing base (c9c8c9d) to head (29e7771).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #433      +/-   ##
==========================================
+ Coverage   86.69%   86.70%   +0.01%     
==========================================
  Files          24       24              
  Lines        3464     3468       +4     
==========================================
+ Hits         3003     3007       +4     
  Misses        461      461              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Apr 09 '25 17:04 codecov[bot]

FYI, the meson build currently packages all files in the subdirs of pyoptsparse/. This includes Python source and compiled modules, but ALSO - README's, Fortran and C source, and anything else which might be in those folders. Some of this stuff probably doesn't belong in the built wheel.

There is likely a better way to do this that doesn't include all the extra stuff - perhaps using some sort of file type filter before running install_subdir?

whophil avatar Apr 10 '25 05:04 whophil

Not sure how to do that exactly, the docs aren't very clear. If we can't figure out a way I don't think it's a deal breaker, think it's also been the case since basically the beginning, so I propose that we merge and deal with it in the future. I did open #434 though and I'd like that merged first, so we can test all the optional installs with mesonpy and make sure that all looks good.

ewu63 avatar Apr 13 '25 02:04 ewu63

@ewu63 this change excludes the source and subdir README files 0dd27fbd423b1143ccb328a7d3876c09131033d5

whophil avatar Apr 14 '25 05:04 whophil

@ewu63 this change excludes the source and subdir README files 0dd27fb

Yeah that looks good, feel free to update this PR

ewu63 avatar Apr 15 '25 04:04 ewu63

Sorry, I haven't been following this. Is this ready from your side? @ewu63 @whophil

kanekosh avatar Jun 02 '25 16:06 kanekosh

FYI, the real issue is dug into an error log dropdown printout:

  Sanity testing Fortran compiler: flang.exe
  Is cross compiler: False.
  Sanity check compiler command line: flang.exe sanitycheckf.f -o sanitycheckf.exe
  Sanity check compile stdout:
  LINK : fatal error LNK1104: cannot open file 'msvcrt.lib'
  
  -----
  Sanity check compile stderr:
  flang.exe: error: linker command failed with exit code 1104 (use -v to see invocation)

I am not familiar with compilers on Windows, but looking at some discussions like this one (or this one - does the Windows build use Visual Studio in the backend?)we might need to update a compiler option or link to the missing library

marcomangano avatar Jul 21 '25 21:07 marcomangano

I think this is finally ready? CC @marcomangano @eirikurj @whophil @kanekosh

ewu63 avatar Aug 14 '25 19:08 ewu63

LGTM 🚀

whophil avatar Aug 15 '25 01:08 whophil

Maybe it is a problem with my machine, but when I try and editable installation I am getting this error:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<frozen importlib._bootstrap>", line 1360, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1322, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 1262, in _find_spec
  File "$HOME/.pyenv/versions/stable/lib/python3.12/site-packages/_pyoptsparse_editable_loader.py", line 311, in find_spec
    tree = self._rebuild()
           ^^^^^^^^^^^^^^^
  File "$HOME/.pyenv/versions/stable/lib/python3.12/site-packages/_pyoptsparse_editable_loader.py", line 345, in _rebuild
    subprocess.run(self._build_cmd, cwd=self._build_path, env=env, stdout=subprocess.DEVNULL, check=True)
  File "$HOME/.pyenv/versions/3.12.10/lib/python3.12/subprocess.py", line 548, in run
    with Popen(*popenargs, **kwargs) as process:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "$HOME/.pyenv/versions/3.12.10/lib/python3.12/subprocess.py", line 1026, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "$HOME/.pyenv/versions/3.12.10/lib/python3.12/subprocess.py", line 1955, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/pip-build-env-bglbkx9w/normal/bin/ninja'

It works with the non-editable installation and I double-checked that the issue is introduced with this PR. I am not familiar with Meson machinery, but it looks like it is trying to do some kind of build at runtime and it is missing the ninja library used at build-time and stored in a temporary folder?

marcomangano avatar Aug 15 '25 17:08 marcomangano

Ah I was wondering if I needed to include ninja, pushed. Thanks for being diligent and checking

ewu63 avatar Aug 15 '25 17:08 ewu63

I am sorry I was unclear, the build works just fine, the error above occurs whenever you try and import pyoptsparse. I am really confused by the fact that the _pyoptsparse_editable_loader.py script generated by meson includes some kind of rebuild on the fly.

marcomangano avatar Aug 15 '25 20:08 marcomangano

I see, it seems that the behaviour here is a little different from setuptools: in an editable install, mesonpy will actually check if the compiled code has been modified, and recompiles as necessarily. This is nice in principle because now the compiled library is also considered "editable", instead of requiring manual deletion of build dir + recompiling. However, it does require the following

  • running pip install -e . with the --no-build-isolation flag, since this secondary compilation step must happen in your dev env -- for some reason mesonpy does not create temporarily build envs for subsequent builds
  • building without isolation means that your dev env must have the build deps installed (which are specified in pyproject.toml). This was the nice thing about setuptools, where it would install build deps for you in a separate build env. This is now done only initially, but in subsequent builds it will have trouble finding the build tools again since that original env has been thrown away.
  • This rebuild nicety is unfortunately not disable-able (see here).

So, I'm not sure what we want to do. I think the dev experience is slightly degraded but not making os.subprocess calls in setup.py seems like a major improvement. I also buy the argument (from meson devs) that editable installs are inherently not isolated builds, so expecting them to be isolated is pointless. We also already require gcc etc. for these installs, so some extra build requirements are not the end of the world if we document these steps properly. What do you all think?

ewu63 avatar Aug 15 '25 21:08 ewu63

I need to look into the discussion a bit more in detail, but I think a practical solution could be enabling something like pip install -e [dev] . and have the build-time dependencies installed in the same environment "on demand". This would avoid future version headaches with our docker stencil and retain the current editing capabilities. As long as we document this, we should be fine.

Since we only (aside minor exceptions) work with python, it feels weird to recompile (even if it is just through pip) as we do code dev, but I agree that we should just find a work around and move on with ditching setuptools as soon as possible.

marcomangano avatar Aug 15 '25 22:08 marcomangano

EDIT: the [dev] approach might also be a problem because we need numpy >= 2.0 at build time

marcomangano avatar Aug 15 '25 22:08 marcomangano

Circling back on this, what do we think? Is this ready to be merged? @marcomangano @whophil @kanekosh @eirikurj I would also probably bump the minor version here.

ewu63 avatar Nov 16 '25 20:11 ewu63

This is a bit old, could somebody sync this with main to see if it still builds?

whophil avatar Nov 17 '25 00:11 whophil

I will check and test again the PR later (apologies for being AWOL as a maintainer), but I just want to double check if I get the editable build instructions right @ewu63 :

If I don't want to upgrade my whole environment to numpy > 2.0, can I temporarily upgrade numpy, build/install the package, and then revert back to numpy 1.2x.xx to run my optimizations? Or should I just give up on the editable install if I still want to use numpy 1 at runtime?

marcomangano avatar Nov 17 '25 17:11 marcomangano

Regardless, I think this PR warrants a minor version bump instead. Given the conversation in #459 and the doc update in #466 (https://github.com/mdolab/pyoptsparse/blob/f7b9c68fbb1790a6d6bec897709188165545030d/doc/install.rst?plain=1#L32) we should probably bump the minimum python version in pyproject.toml and environment.yml

marcomangano avatar Nov 17 '25 18:11 marcomangano

If I don't want to upgrade my whole environment to numpy > 2.0, can I temporarily upgrade numpy, build/install the package, and then revert back to numpy 1.2x.xx to run my optimizations? Or should I just give up on the editable install if I still want to use numpy 1 at runtime?

I think that should work but not 100% sure. In general I caution against editable installs unless you are actually developing stuff in pyOptSparse.

I found a quirky issue with editable installs. Because meson will use a separate build dir to store the compiled .so libraries (i.e. out-of-tree build) and rely on some import hook to manage this, our import_module function did not work as it expects the library to be exactly where we put it. I pushed a little hack which worked for me but is definitely fragile, we can probably improve it by either doing some relative path calculations, or more explicit in all the conditions we want to try the import:

  1. import slsqp from this exact path (either the actual path for in-source builds or special SNOPT path var)
  2. import pyoptsparse.pySLSQP.slsqp from sys.path <- this is what we need with editable installs + mesonpy

Thoughts @whophil ?

EDIT: the [dev] approach might also be a problem because we need numpy >= 2.0 at build time Not necessarily. It is preferred but if you just stick to numpy1 on your local machine then it will still work. The binaries will not work if you subsequently install numpy2 but you should not expect that to work. Any compat issue with numpy1/2 will be caught by CI so I'm not too worried.

I am OK with adding a separate group of optional deps.

ewu63 avatar Nov 19 '25 07:11 ewu63