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

Enable user requested termination of IPOPT #305

Merged
merged 6 commits into from
Oct 6, 2022

Conversation

swryan
Copy link
Contributor

@swryan swryan commented Sep 27, 2022

Purpose

The intent of this change is to enable the IPOPT Optimizer to respond to a user requested termination in the same way that the SNOPT Optimizer does.

As with SNOPT, a value of 2 returned in the fail flag from a user evaluation function will be interpreted as a request to terminate the optimization. This is accomplished by means of the intermediate_callback in the IPOPT API.

Expected time until merged

Not urgent.

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

The existing user_termination test for SNOPT has been updated to test IPOPT as well.

Checklist

  • I have run flake8 and black to make sure the Python code adheres to PEP-8 and is consistently formatted
    • I ran black and flake8 but left a few existing lines that flake8 complained were over 80 columns
    • my black made some changes that your black didn't like
  • 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

@swryan swryan requested a review from a team as a code owner September 27, 2022 18:56
@swryan swryan requested review from ewu63 and sseraj September 27, 2022 18:56
@codecov
Copy link

codecov bot commented Sep 27, 2022

Codecov Report

Merging #305 (ad3ea6b) into main (7b98186) will decrease coverage by 11.64%.
The diff coverage is 86.66%.

@@             Coverage Diff             @@
##             main     #305       +/-   ##
===========================================
- Coverage   84.17%   72.53%   -11.65%     
===========================================
  Files          22       22               
  Lines        3317     3331       +14     
===========================================
- Hits         2792     2416      -376     
- Misses        525      915      +390     
Impacted Files Coverage Δ
pyoptsparse/pyIPOPT/pyIPOPT.py 95.83% <86.66%> (-1.34%) ⬇️
pyoptsparse/pySNOPT/pySNOPT.py 12.92% <0.00%> (-76.93%) ⬇️
pyoptsparse/pyNLPQLP/pyNLPQLP.py 25.68% <0.00%> (-66.98%) ⬇️
pyoptsparse/pyOpt_solution.py 57.77% <0.00%> (-42.23%) ⬇️
pyoptsparse/pyOpt_utils.py 60.46% <0.00%> (-7.91%) ⬇️
pyoptsparse/pyOpt_history.py 77.22% <0.00%> (-5.80%) ⬇️
pyoptsparse/pyOpt_optimizer.py 83.40% <0.00%> (-1.82%) ⬇️
pyoptsparse/pyOpt_optimization.py 78.19% <0.00%> (-0.81%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ewu63
Copy link
Collaborator

ewu63 commented Sep 27, 2022

Hmm seems to be a linking issue on the Windows build. I'm not really familiar with conda nor flang but maybe @jackm97 is able to take a look? The log says

The Meson build system
Version: 0.63.2
Source dir: D:\a\pyoptsparse\pyoptsparse
Build dir: D:\a\pyoptsparse\pyoptsparse\meson_build
Build type: native build
Project name: pyoptsparse
Project version: undefined
C compiler for the host machine: cl (msvc 19.16.27048 "Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27048 for x64")
C linker for the host machine: link link 14.16.27048.0
C++ compiler for the host machine: cl.exe (msvc 19.16.27048 "Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27048 for x64")
C++ linker for the host machine: link link 14.16.27048.0
Host machine cpu family: x86_64
Host machine cpu: x86_64
Library m found: NO

meson.build:31:0: ERROR: Unable to detect linker for compiler `flang -Wl,--version`
stdout: LINK : warning LNK4044: unrecognized option '/-version'; ignored
LINK : warning LNK4001: no object files specified; libraries used
LINK : warning LNK4068: /MACHINE not specified; defaulting to X64
flangmain.lib(flangmain.c.obj) : error LNK2019: unresolved external symbol MAIN_ referenced in function main
a.exe : fatal error LNK1120: 1 unresolved externals

stderr: flang.exe: error: linker command failed with exit code 1120 (use -v to see invocation)

This isn't necessarily a blocker for me and I am okay with merging this if fixing this takes too long. It appears to be an unrelated issue anyhow.

Copy link
Contributor

@marcomangano marcomangano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good to me, approving to speed up the process but I agree we should look into that windows failure

@ewu63
Copy link
Collaborator

ewu63 commented Oct 6, 2022

The issue seems to be the same as mesonbuild/meson#10778, so I will merge this PR now and deal with the Windows build failure later.

@ewu63 ewu63 merged commit ee46c12 into mdolab:main Oct 6, 2022
@jackm97
Copy link
Contributor

jackm97 commented Oct 8, 2022

@nwu63 Sorry, I just saw I was mentioned in this thread. Is there still an issue with the Windows build?

@ewu63
Copy link
Collaborator

ewu63 commented Oct 8, 2022

@jackm97 no worries. Yes the Windows build still fails but the issue was reported to meson in the issue referenced above. Do you know if we could use a different Fortran compiler on Windows instead of flang? I thought conda provides GCC for Windows but I'm not sure.

@swryan swryan changed the title Enable user requested termination if IPOPT Enable user requested termination of IPOPT Oct 8, 2022
@jackm97
Copy link
Contributor

jackm97 commented Oct 10, 2022

@nwu63 It does have GCC, the problem is it's extremely out of date. They're on something like gcc 4.6. I now recall this issue when building pyoptsparse internally. We locked meson to a previous version in the environment.yml. It slipped my mind that we did this.

Is that a good enough workaround until it's fixed in meson?

Edit to add: We may also be able to specify the linker, rather than let meson guess

@ewu63
Copy link
Collaborator

ewu63 commented Oct 10, 2022

@jackm97 I don't want to take up too much of your time, so if it's easier to just pin meson to an earlier version let's do that. Would you mind pushing a small PR to test that out?

@jackm97
Copy link
Contributor

jackm97 commented Oct 11, 2022

@nwu63 will do

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.

None yet

4 participants