Skip to content

Conversation

jameshammondpx
Copy link

Description

Implements callbacks in scipy_interface wrapper of cyipopt.Problem, addressing the Issue raised in #290

Callbacks should be defined as a callable callback(xk), where xk is the current parameter vector, consistent with: https://docs.scipy.org/doc/scipy/reference/generated/scipy.optimize.minimize.html.

Development Notes

  • Refactors IpoptProblemWrapper to subclass cyipopt.Problem
  • Enables use of self.get_current_iterate() in intermediate to pass the current candidate xk to the callback
  • Docstrings updated accordingly

@moorepants
Copy link
Collaborator

Thanks for working on this. We'll need to keep the __init__ method backwards compatible, so you'll need to think of an alternative way of implementing this. We'll need some unit tests to check the functionality also.

@jameshammondpx
Copy link
Author

Thanks, have you got any ideas for keeping the __init__ method backwards compatible? I will look at adding some unit tests.

One idea for backwards compatibility is to:

  • Make n,m,lb,ub,cl,cu optional arguments to the constructor
  • Only call super.__init__() if those arguments are passed
  • Only allow callbacks if those arguments are passed

But it's not particularly elegant. The issue is self.get_current_iterate() is required to get information for the callback, but is only defined in cyipopt.Problem.

@moorepants
Copy link
Collaborator

Can Problem be an internal attribute/variable? (instead of subclassing)?

@moorepants
Copy link
Collaborator

I see that the scipy code creates a Problem without subclassing Problem, is it that the call backs require a subclass of Problem? Maybe making a second IpoptWrapper class based on the subclass would avoid the backwards incompatibilities.

@jameshammondpx
Copy link
Author

I've created a new IpoptProblem class which subclasses Problem and: (1) instantiates a IpoptProblemWrapper, (2) overrides the default intermediate method. This way IpoptProblemWrapper remains untouched.

Interested to hear your thoughts.

@jameshammondpx
Copy link
Author

I've created a new IpoptProblem class which subclasses Problem and: (1) instantiates a IpoptProblemWrapper, (2) overrides the default intermediate method. This way IpoptProblemWrapper remains untouched.

Interested to hear your thoughts.

@moorepants - what do you think?

@moorepants
Copy link
Collaborator

I think this is fine.

@moorepants
Copy link
Collaborator

Can you merge in master and lets see if the tests run?

@moorepants
Copy link
Collaborator

I merged master and pushed.

@moorepants
Copy link
Collaborator

This error occurs FAILED cyipopt/tests/unit/test_scipy_optional.py::test_gh115_eps_option - AttributeError: 'IpoptProblem' object has no attribute 'nfev'.

@moorepants
Copy link
Collaborator

get_current_iterate() is only available for Ipopt >=3.14, so you'll need to make this compatible with Ipopt 3.13 also.

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.

2 participants