Distribute Ipopt library and headers that can be used with CyIpopt

[Robbybp]

I'd like to use CyIpopt because of its potential to help with debugging, and it would be nice to use it with IDAES-provided binaries. CyIpopt needs an Ipopt shared library and Ipopt include files. The Ipopt shared library that we distribute should work, and it shouldn't be too hard to distribute the include files in an .idaes/include directory. I can patch CyIpopt to use our Ipopt library, which gives me the following link step (on MacOS) when running python setup.py develop for CyIpopt:

clang -bundle -undefined dynamic_lookup -arch arm64 -arch x86_64 -Wl,-headerpad,0x1000 build/temp.macosx-10.9-universal2-3.9/cyipopt/cython/ipopt_wrapper.o -L/Users/robert/.idaes/bin -lipopt -o /Users/robert/cyipopt/ipopt_wrapper.cpython-39-darwin.so

This seems to successfully compile the ipopt_wrapper.cpython-39-darwin.so library that CyIpopt uses, but trying to use this (e.g. run the CyIpopt tests) gives me the following run-time error:

E   ImportError: dlopen(/Users/robert/cyipopt/ipopt_wrapper.cpython-39-darwin.so, 0x0002): Library not loaded: /Users/john/src/idaes-ext-build/coinbrew/dist-share/lib/libipopt.3.dylib
E     Referenced from: <3E860C76-3127-3257-A4B6-8841E9BD66A3> /Users/robert/cyipopt/ipopt_wrapper.cpython-39-darwin.so
E     Reason: tried: '/usr/local/lib/libipopt.3.dylib' (no such file), '/libipopt.3.dylib' (no such file), '/Users/robert/.pyomo/lib/libipopt.3.dylib' (no such file), '/Users/john/src/idaes-ext-build/coinbrew/dist-share/lib/libipopt.3.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/Users/john/src/idaes-ext-build/coinbrew/dist-share/lib/libipopt.3.dylib' (no such file), '/Users/john/src/idaes-ext-build/coinbrew/dist-share/lib/libipopt.3.dylib' (no such file), '/usr/local/lib/libipopt.3.dylib' (no such file), '/usr/lib/libipopt.3.dylib' (no such file, not in dyld cache)

When I inspect the CyIpopt library with otool -l ipopt_wrapper.cpython-39-darwin.so, I see the following load step:

Load command 10
          cmd LC_LOAD_DYLIB
      cmdsize 104
         name /Users/john/src/idaes-ext-build/coinbrew/dist-share/lib/libipopt.3.dylib (offset 24)
   time stamp 2 Wed Dec 31 17:00:02 1969
      current version 17.2.0
compatibility version 17.0.0

I am not entirely sure why CyIpopt is trying to follow this path for linking. Could this be due to a missing -fPIC flag somewhere? I have not been able to reproduce this behavior compiling Ipopt locally (even when I move the compiled Ipopt files to a new location before linking).

I will continue to investigate, but wanted to post here in case this rings a bell for anyone and so I can reference it later.

[Robbybp]

According to @jsiirola, this is because the path /Users/john/src/idaes-ext-build/coinbrew/dist-share/lib/libipopt.3.dylib is included in the pkg-config file we distribute. I'm not sure how this is getting picked up in the compilation above, because IIRC I'm just linking the libipopt.dylib file that we distribute and bypassing pkg-config (I get the include files directly from a local Ipopt install). I'll double check when I'm back on that computer.

[Robbybp]

The problem I encountered seems to occur because the "install name" of the library we distribute on MacOS is set to some path on John Eslick's machine/VM. Luckily, we can change this pretty easily:

install_name_tool -id $HOME/.idaes/lib/libipopt.3.dylib $HOME/.idaes/lib/libipopt.dylib
install_name_tool -id $HOME/.idaes/lib/libipopt.3.dylib $HOME/.idaes/lib/libipopt.3.dylib

with this, I can get CyIpopt working with IDAES-distributed libraries and header files.

I also don't have $HOME/.idaes/lib on my LD_LIBRARY_PATH or DYLD_LIBRARY_PATH. I could probably have fixed this just by adding this to the dyld path.

I'm not sure what the right way to distribute a linkable-on-MacOS Ipopt library is. (E.g. make users add $HOME/.idaes/lib to their DYLD_LIBRARY_PATH or run install_name_tool on these libraries as part of idaes get-extensions.) It seems like the .dylib file format has some tokens (e.g. @rpath) that might be useful? (See https://github.com/qyang-nj/llios/blob/main/macho_parser/docs/LC_dylib.md)

[Robbybp]

For what it's worth, the Pyomo-compiled libpynumero_MA27.dylib library has an "install name" that is @rpath/libpynumero_MA27.dylib. I don't think we set this explicitly (via ld -install_name @rpath/<libname>) for either the Pyomo or IDAES libraries, so I'm not sure why they are being set differently. (Could be a missing -fPIC flag?)

[Robbybp]

Accomplishing this will require modifications to two repos:

• Update idaes get-extensions (or possibly some other option of the idaes command) to unpack the idaes-lib-<os>.tar.gz and idaes-local-<os>.tar.gz files. This seems to happen here, although I'm not sure exactly what will have to change at this point.
• Update the distributed extensions (specifically, libipopt and ipopt.pc) to not reference specific paths on John E.'s machine. The libipopt modification may only be relevant on MacOS. This happens in this repo, but again I'm not entirely sure what will need to change.

Then, the user-facing instructions to install CyIpopt with our IDAES binaries will be:

pip install idaes
idaes get-extensions
export PKG_CONFIG_PATH=$PKG_CONFIG_PATH:$HOME/.idaes/lib/pkgconfig
pip install cyipopt

(They may also need to set LD_LIBRARY_PATH to include .idaes/lib in order to find the library at run-time. This isn't necessary on MacOS with -install_name @rpath/<libname> during compilation, but I haven't tested on Linux or Windows.)

[Robbybp]

Testing the changes in #262, I think the user will need to set DYLD_LIBRARY_PATH on MacOS in order for the @rpath install names to work (i.e. for CyIpopt to find libipopt.dylib at run-time).

[ksbeattie]

@Robbybp same question here - this for an end-of-Nov release?

[Robbybp]

@ksbeattie This requires #262 plus and IDAES PR for idaes command updates. It will probably be completed one week behind #262, if no issues with the command updates pop up.

[Robbybp]

Changes to make the libraries linkable have been merged in #262. @bpaul4 @MarcusHolly Can you work on a release of the Windows and Linux builds?

[MarcusHolly]

Changes to make the libraries linkable have been merged in #262. @bpaul4 @MarcusHolly Can you work on a release of the Windows and Linux builds?

So we just need to generate the tar files, right? Just as a sanity check, I think I should remake the ones I've generated (from #262) a few weeks ago. I should be able to have this done by some point tomorrow

[Robbybp]

Yes, this should be as easy as uploading the tar files from the build process to the releases page. Yes, I would recommend you remake the tar files for sanity. Thanks! Let me know if you run into any unexpected issues.

[MarcusHolly]

@Robbybp

I was able to generate the tar files without any issues for the Windows and Linux builds. I followed the naming and tag conventions for pre-releases in the IDAES repo, but I'm not sure what the description should be for the release.

A few other questions/comments:

1.) Should I publish it as a pre-release or a regular release?

2.) I noticed previous releases in idaes-ext include tar files for darwin. What are these? The Mac files?

3.) Lastly, I've just realized that I only generated the tar files for x86_64. I tried generating the aarch64 files, but I don't know if it'll even work on my laptop...

[MarcusHolly]

This is the error I get when trying to run aarch64

[Robbybp]

1.) Should I publish it as a pre-release or a regular release?

I think pre-release makes more sense.

2.) I noticed previous releases in idaes-ext include tar files for darwin. What are these? The Mac files?

Yes. These are for Mac. You'll need a Mac to compile these. My current plan is that I will compile these separately (without HSL), which we can use for testing until you or Brandon have access to a Mac.

3.) Lastly, I've just realized that I only generated the tar files for x86_64. I tried generating the aarch64 files, but I don't know if it'll even work on my laptop...

I believe this is expected. Our docker images are not set up to build cross-architecture. I'm not sure why the docker build scripts accept the architecture as an argument. You will need an ARM machine to build the aarch64 files. I believe this is typically done from an apple-silicon Mac. Assuming you don't have access to an ARM machine, I can build these binaries without HSL.

My recommendation is to make a "partial" pre-release with just the files you can build, then I will send you the binaries I am responsible for (all ARM builds without HSL), which you can include in a follow-up pre-release.

but I'm not sure what the description should be for the release

Just make it very clear that this is a partial, WIP release for integration testing purposes that is not intended for general use.

[MarcusHolly]

@Robbybp I ran the Windows test via GitHub actions on the 3.5.0beta pre-release tag: https://github.com/IDAES/idaes-ext/actions/runs/7279400306. It failed pretty quickly...

[lbianchi-lbl]

@Robbybp I ran the Windows test via GitHub actions on the 3.5.0beta pre-release tag: https://github.com/IDAES/idaes-ext/actions/runs/7279400306. It failed pretty quickly...

I believe the Python version being used is too old. IDAES requires Python 3.8+.

[Robbybp]

Yep just opened a PR to update this (somewhat arbitrarily) to 3.10 #265.

[MarcusHolly]

Will I have to rebuild all the binaries again or is it enough to just re-run the test after #265 is merged?

[Robbybp]

You should not need to re-build the binaries

[MarcusHolly]

@Robbybp I've merged in the Python update PR, but the same failure occurs with node12 being deprecated. I'm assuming Python 3.10 is supposed to run on node16, so maybe I do need to rebuild the binaries?

[Robbybp]

With what I'm seeing here, it looks like we were still trying to set up Python 3.7. Not sure what the reason could be. Or are you referring to a different failure?

[Robbybp]

I just pushed a beta release that includes the ARM binaries for Mac, Ubuntu, and RHEL8. This should be sufficient for testing updates to idaes get-extensions, and for testing CyIpopt against these libraries. However, for the final 3.5.0 release, we will want binaries for all platforms compiled and tested with HSL.

[MarcusHolly]

So it seems like GitHub Actions still defaults to Python 3.7, even though we've updated test_windows_main.yml. I'm not quite sure how to get around this issue.

[MarcusHolly]

Small update on the GitHub actions front:

I ended up re-making the binaries, which addressed Python version issue from the comment above. Currently, the windows build is passing in GitHub actions but there are a few errors in the Linux builds. I'm not sure how those should be addressed...

[MarcusHolly]

@Robbybp Thoughts on the above comment?

[Robbybp]

I haven't had time to look at the errors. I hit some segfaults testing our Mac builds with CyIpopt and Mumps that I've been trying to isolate. I hope to get to the GHA Linux errors early next week.

[Robbybp]

Looking briefly, it is only two test failures on a tray column solve. TBH I am more encouraged that the rest of the tests pass, given that it is our first time going through this build process (although these failures should still be addressed). Looking at the last linux tests 7 months ago, https://github.com/IDAES/idaes-ext/actions/runs/5212024395, the currently failing platforms (rocky8, ubuntu18, and debian10) all passed, although a few other OSs failed at that time. Unfortunately, the old test logs have expired. I will prioritize testing the Mac build with Mumps for now, and we can discuss the test failures with the group next Thursday.

[bknueven]

We have an emerging use-case for CyIpopt in WaterTAP. Is there anything I can do to push this along?

[bknueven]

@Robbybp following your advice above, I was able to get cyipopt to compile against the IDAES distribution by manually downloading the package, setting PKG_CONFIG_PATH as needed, and making the below modification to the ipopt.pc file.

15d14
< Requires.private: coinhsl coinmumps 

Mostly putting it here so I can return to it later.

[adowling2]

My memory may be a little rusty, but a few months ago, I got cyipopt to install via conda. After installing cyipopt, I also installed the HSL libraries for macOS-arm. (I have an academic license.)

However, I think had a seg fault issue, which may have been due to something else. I then switched to a Linux machine: CCSI-Toolset/measurement-optimization#2

I'm sharing these details here while I remember them. Sorry, theses may not be that helpful.

[ksbeattie]

We have an emerging use-case for CyIpopt in WaterTAP. Is there anything I can do to push this along?

@bknueven I've placed this on the Aug release board to raise awareness of it during dev calls. Currently though we don't have a clearly identified and dedicated owner of the idaes-ext builds. Hopefully that will change soon, with perhaps someone at Sandia taking it over.

[Robbybp]

With the release of idaes-ext 3.4.2, these binaries can be used to build CyIpopt. I have tested this on Github actions Linux, Mac, and Windows runners here, and it works on all tested platforms at the time of this comment.

Getting CyIpopt to work with our binaries still involves several manual steps. This is the process for Mac and Linux. The process for Windows is more involved, and involves a custom branch of CyIpopt. I'm not sure what we'll do about this going forward.

1. Unpack the idaes-local tar.gz file.
2. Add .../idaes-local/lib/pkgconfig to PKG_CONFIG_PATH.
3. Install CyIpopt (pip install cyipopt, or download source and install with setup.py or pip)
4. Put .../idaes-local/lib on LD_LIBRARY_PATH (Linux) or DYLD_LIBRARY_PATH (Mac). Note that this has to be done before starting the Python process. There may be some way to dynamically change the runtime linker path (that the CyIpopt shared library uses) from within a Python process, but I don't know what it is.
5. Run your Python code that uses CyIpopt.

We could automate this to make it a bit more user-friendly. See my initial attempt here.

[Robbybp]

The rest of the work that needs to happen here is automation and documentation that happens in IDAES-PSE, so I'm going to close this issue.