IDAES / idaes-pse

The IDAES Process Systems Engineering Framework
https://idaes-pse.readthedocs.io/
Other
218 stars 235 forks source link

[WIP] Add a command to install CyIpopt #1474

Closed Robbybp closed 1 month ago

Robbybp commented 3 months ago

Fixes

In conjuction with #1473, addresses part of https://github.com/IDAES/idaes-ext/issues/261. (Not sure if this works on Windows...)

Summary/Motivation:

CyIpopt needs to be able to find a copy of the Ipopt shared library and header files, which it does (at least on Linux and Mac) using pkg-config. To install with Ipopt in a non-standard location (such as IDAES's Ipopt), we need to set the PKG_CONFIG_PATH environment variable (to include .idaes/bin/lib/pkgconfig, which will likely change in new idaes-ext releases). To make this a little bit easier for users, I propose to add a command to install CyIpopt in the current Python environment, e.g.:

idaes install-cyipopt

This basically just runs pip install cyipopt with PKG_CONFIG_PATH set appropriately.

I believe users will still need to set LD_LIBRARY_PATH (or DYLD_LIBRARY_PATH/PATH on Mac/Windows) to actually use CyIpopt. import idaes does set these variables, but I believe import cyipopt uses the value of the variables at the time the Python process started, not the current values in os.environ.

This will only work with idaes-ext 3.4.2

Changes proposed in this PR:

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.
codecov-commenter commented 3 months ago

Codecov Report

Attention: Patch coverage is 16.00000% with 42 lines in your changes missing coverage. Please review.

Project coverage is 76.33%. Comparing base (c2825ca) to head (93aa362). Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
idaes/commands/extensions.py 12.50% 35 Missing :warning:
idaes/commands/util/download_bin.py 0.00% 7 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1474 +/- ## ========================================== - Coverage 76.38% 76.33% -0.06% ========================================== Files 394 394 Lines 65121 65167 +46 Branches 14427 14436 +9 ========================================== + Hits 49745 49747 +2 - Misses 12813 12858 +45 + Partials 2563 2562 -1 ```

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

Robbybp commented 3 months ago

Marking as WIP for now. As it stands, this may not be worth it compared to the alternative of simply documenting all the steps that are necessary to build CyIpopt with IDAES binaries. If we could do everything in a single command, I think it would be worth it. The barrier to this is loading our IPOPT library without relying on a library path from the environment. This may be possible with a patch to CyIpopt? Also, this command should be tested in our CI for Mac, Linux, and Windows.

Robbybp commented 1 month ago

Closing as I'm not actively working on this. Will re-open if it looks like this will be worth the effort.