-
Notifications
You must be signed in to change notification settings - Fork 37
Update jac_lin_coord methods
#501
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
Conversation
|
@arnavk23 It is a breaking change if we drop the signature with |
tmigot
left a comment
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.
Thanks @arnavk23 for the PR.
Can you also adapt jac_lin_op and jac_lin_op! https://github.com/arnavk23/NLPModels.jl/blob/504a3c29574998b892d7f598283db466a86e391e/src/nlp/api.jl#L737
|
@tmigot All test have passed except the breakage/upload one. |
tmigot
left a comment
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.
Thanks @arnavk23 for these changes. I gave some thoughts about @amontoison 's comment, and it's a good idea to deprecate these functions instead of removing them directly.
Using the macro @deprecate when somebody uses the "old" variant it should receive a warning saying that the function is deprecated and point to the variant.
tmigot
left a comment
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.
Thanks @arnavk23 for the changes. I made some comments.
I think we also miss the functions jprod and jtprod
Co-authored-by: Tangi Migot <[email protected]>
Co-authored-by: Tangi Migot <[email protected]>
Co-authored-by: Tangi Migot <[email protected]>
Co-authored-by: Tangi Migot <[email protected]>
Co-authored-by: Tangi Migot <[email protected]>
Co-authored-by: Tangi Migot <[email protected]>
Co-authored-by: Tangi Migot <[email protected]>
Co-authored-by: Tangi Migot <[email protected]>
|
We also need to remove the |
Co-authored-by: Tangi Migot <[email protected]>
Co-authored-by: Tangi Migot <[email protected]>
Co-authored-by: Tangi Migot <[email protected]>
Co-authored-by: Tangi Migot <[email protected]>
Co-authored-by: Tangi Migot <[email protected]>
tmigot
left a comment
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.
Thanks @arnavk23 !
This reverts commit 3091fb0.
This reverts commit 3091fb0.
The linear Jacobian part is independent of
x, so passing it became unnecessary.jac_lin_coord(nlp, x)→jac_lin_coord(nlp)jac_lin_coord!(nlp, x, vals)→jac_lin_coord!(nlp, vals)Closes #404