Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Ipopt 3.13 support for Windows #63

Merged
merged 4 commits into from
Sep 24, 2020

Conversation

apommel
Copy link
Contributor

@apommel apommel commented Jun 3, 2020

Issue #62

This updates the setup.py files to work with precompiled Ipopt 3.13.XX binaries for Windows (tested with Ipopt 3.13.2 from their Github). As explained in the Issue thread, this breaks compatibility with older Ipopt precompiled versions because of the different structure. According to feedback, a environment variable could be introduced to specify the Ipopt version.

Edit: I don't know if this has to do with the new Ipopt build system, but this works equally on anaconda and python.org Python with the dependencies installed via pip.

@apommel
Copy link
Contributor Author

apommel commented Jun 4, 2020

Sorry for the numerous commits. I initially expected AppVeyor to fail because I broke backward compatibility without updating it initially, but it actually failed because of another issue related to conda environment activation. It is fixed now.
Travis also fails, but it is unrelated to my PR. It seems that pkg-config is not set up correctly with the latest Ipopt versions that conda downloads (3.13.2). It should be resolved in another PR, which could update the installation instructions for Linux with Ipopt 3.13 at the same time.

@apommel
Copy link
Contributor Author

apommel commented Jun 5, 2020

I will probably try to rebase the PR against the current release (never done that before). Are there any change necessary before that @moorepants ?

@moorepants
Copy link
Collaborator

You are good to go. I've bumped to 0.3.0.dev0

@apommel apommel force-pushed the 3.13-win-support branch from 94ab5d3 to e070a4b Compare June 5, 2020 01:22
@moorepants
Copy link
Collaborator

What versions of IPOPT will we be supporting on each OS? Or is this 3.13 min version only necessary for Windows?

@apommel
Copy link
Contributor Author

apommel commented Jun 5, 2020

@moorepants Linux and MacOS will not have hard requirements on Ipopt version, but the install instructions will likely be different, at least when building from source. The requirements depends if we maintain version specific build instructions in the readme (although the best would be to have the conda install work again for all versions).

@apommel apommel mentioned this pull request Jul 2, 2020
7 tasks
@moorepants
Copy link
Collaborator

@brocksam is asking about this PR in #14 (comment)

I don't recall the status here, but it looks like @apommel was going to rebase. But I'm not sure that is needed.

From my perspective, bumping the dependency of IPOPT to a higher version is fine, but we just need to update all the documentation and related installation files to reflect that. I have forgotten the details, but if @apommel has some time to chime in and update this, I can review.

@brocksam, you are also welcome to complete this PR if you know what is needed.

@apommel
Copy link
Contributor Author

apommel commented Sep 24, 2020

From what I remember, I considered it finalized. I had rebased to make the commits clearer, and had updated the documentation accordingly with the changes. Is anything more needed @moorepants ? In my opinion it is ready to be merged since early June.

@moorepants
Copy link
Collaborator

Got it. I didn't know it was updated.

@moorepants moorepants merged commit ada3198 into mechmotum:master Sep 24, 2020
@brocksam
Copy link
Collaborator

Excellent, thanks very much guys! Are there plans for a 0.3.0 release in the near future? Would it be helpful if I made a PR (similar to the one @moorepants did for 0.2.0, #65) so that we can get these new changes on conda-forge?

@moorepants
Copy link
Collaborator

Yes, a PR would help. I have no immediate plans, but one issue is that the whole library might need to be updated for this new ipopt version that has a different interface and/or build system. This PR only addressed Windows and we have Mac and Linux to sort out.

@brocksam
Copy link
Collaborator

I'm already successfully using Cypopt on macOS and Linux with Ipopt 3.13.2 (same as the Windows version used here) so with this PR it should now be a universal version across the board. In which case I think there may be limited changes that need to be made before a 0.3.0 release can be made, with the motivation for 0.3.0 release being to provide easy installation of Cyipopt supporting Ipopt 3.13.X on Windows via conda-forge etc. I have submitted a draft PR (#74) so we have a more logical place to discuss this further.

brocksam added a commit to brocksam/cyipopt that referenced this pull request Sep 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants