OpenMDAO / build_pyoptsparse

python script to build/install pyoptsparse with IPOPT (and optionally SNOPT)
Apache License 2.0
9 stars 15 forks source link

Fix HSL build issues #62

Closed harris-josh closed 9 months ago

harris-josh commented 9 months ago

Summary

This PR fixes several build issues when building pyOptSparse with Ipopt and HSL solvers.

The configuration logic for the Mumps and HSL builds was largely the same, so I refactored the common parts into a get_common_solver_config_cmd function in order to remove duplication; YMMV on whether this is a good change or not.

This function sets up the arguments for the configure command and in particular:

  1. Checks to see if -lm needs to be explicitly included. I based this check on whether the OS was Linux or not; this is probably not the most reliable approach and probably merits changing. (It also probably doesn't hurt to just always include -lm?)
  2. If the compiler is identified as Apple Clang the --disable-openmp flag is provided. It's possible to build with OpenMP support, but I figured that wasn't worth it for this (see the "Notes" section below).

I added a new key to the sys_info dictionary, gcc_is_apple_clang, that is true if the compiled is detected as Apple clang via gcc --version and false otherwise since Apple aliases gcc and g++ to clang and clang++ on macOS by default. I added the logic for this flag to the select_*_compilers functions.

For the HSL build, the logic to determine the directory name hard-coded the column of the name in the tar output, which depends on the tar implementation used. This was not working on later versions of Ubuntu. Accordingly, the PR replaces the subprocess.run call to tar and subsequent parsing with direct use of the tarfile module from the Python standard libary.

I tested this with Mumps as the linear solver:

./build_pyoptsparse.py -e -v -f 

and HSL as the linear solver:

./build_pyoptsparse.py -e -l hsl -t ./coinhsl-YYYY-MM-DD.tar.gz -v -f

on both Ubuntu 22.04 and macOS and both built successfully.

Related Issues

Backwards incompatibilities

None

New Dependencies

The script now imports tarfile, but this is a part of the Python standard library and isn't really a new external dependency.

Notes

It's possible to build Ipopt with Apple clang and OpenMP, but it requires the following:

  1. OpenMP must be installed via Homebrew or a similar tool.
  2. CPPFLAGS must be include -Xpreprocessor -fopenmp
  3. CFLAGS and CXXFLAGS must include "-I$OMP_PATH/include"
  4. LFLAGS must include "-Wl,-rpath,$OMP_PATH/lib -L$OMP_PATH/lib -lomp"

The value of OMP_PATH above depends on the method of installation but is probably /opt/homebrew/opt/libomp for Homebrew on Apple Silicon.

I believe I also had to manually fix the output of the coinhsl.pc file for Ipopt when I last built it with OpenMP support on macOS.

swryan commented 9 months ago

would you mind merging out with the latest master branch changes to trigger the test workflow?

also, if there are any test configurations you think should be covered there to demonstrate your fixes, it could be good to add them.

swryan commented 9 months ago

would you mind merging out with the latest master branch changes to trigger the test workflow?

thanks!

harris-josh commented 9 months ago

would you mind merging out with the latest master branch changes to trigger the test workflow?

I rebased on the lastest master and force-pushed.

also, if there are any test configurations you think should be covered there to demonstrate your fixes, it could be good to add them.

Will do. One thing we've seen with macOS is that Ipopt seems to segfault when built with HSL (using a modified version of the Dymos double integrator example as a test). Runs fine with Mumps or with HSL on Ubuntu. I think that's another issue that's out of scope for this PR though (probably a problem with pyOptSparse itself?).

harris-josh commented 9 months ago

Instead of parsing the output of tar I now inspect the tarfile with the Python standard library tarfile module to get the folder name:

with tarfile.open(opts['hsl_tar_file'], 'r') as tf:
    hsl_dir_name = tf.getnames()[0]

This should always return the directory name (assuming a valid input tarball) and avoid any issues with the regular expression based approach.

Also, I plan to add to the test matrix to have versions of the build that force compilation instead of using pre-compiled versions.

harris-josh commented 9 months ago

I added another three items to the workflow matrix to use the --force-build flag for Ubuntu, Ubuntu latest, and macOS.

I disabled MPI for these builds as they would not complete due to a missing metis.h (see this output for more info. I suspect the cause is the temporary folder with the METIS install getting cleaned up between stages but I am not familiar enough with GitHub Actions to say for certain.

There's probably a better way to add to the test matrix (I was hoping to not have to duplicate all the version numbers, for example) but again I'm pretty new to Actions and I'm not familiar with best practices yet.

I thought about a way to test an HSL build but couldn't think of one that I could implement due to licensing concerns.

swryan commented 9 months ago

Thank you for this contribution šŸ‘šŸ¼