-
Notifications
You must be signed in to change notification settings - Fork 56
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
Refactor objective
, gradient
and evaluate_fun_with_grad
methods
#128
Conversation
Can you provide more information? I'm not sure what this is doing or what you want to deprecate. |
Sure! While discussing my last PR, @brocksam proposed to refactor the relevant parts of the cyipopt/cyipopt/scipy_interface.py Lines 107 to 118 in dc17c81
So there's no need to check whether the constraint function returns both the constraint function value and the jacobian inside the This PR does the same for the objective. That is why we no longer need to call |
I think we can just leave the |
@@ -60,8 +60,6 @@ class IpoptProblemWrapper(object): | |||
for more information. | |||
eps : float, optional | |||
Epsilon used in finite differences. | |||
n : int, optional | |||
Total number of variables. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It isn't clear why this is removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should have been removed in this commit in the last PR. Sorry for the mess!
This small PR is motivated by the discussion here. It refactors the
evaluate_fun_with_grad
,objective
andgradient
methods in the scipy interface and handles thejac=True
option for the objective function similar to the constraints by using theMemoizeJac
decorator.Should we leave the
evaluate_fun_with_grad
method as it is, or do you think deprecating it is the better way?