Add OptimizationMadNLP #1061
Merged
Add OptimizationMadNLP #1061
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SebastianM-C
force-pushed
the
smc/madnlp
branch
2 times, most recently
from
14ce037 to
90f20ed
Compare
| lagrangian, soadtype, x, Constant(one(eltype(x))), | ||
| Constant(ones(eltype(x), num_cons)), Constant(p), strict = Val(false)) | ||
| lag_hess_prototype = zeros(Bool, num_cons, length(x)) | ||
| lag_hess_prototype = zeros(Bool, length(x), length(x)) |
There was a problem hiding this comment.
Isn't the Hessian a square n*n matrix with n being the length of the vector we're taking derivatives against? All the tests that I've added error with out of bounds memory access otherwise. The tests don't catch this because we don't actually call with the generated prototype.
|
MadNLP tests failing |
|
yeah, it's due to the bug fixed in 90b5b1c, there are several bugs that lead to CI failures on the latest releases, which is why I was wondering if I should split those to a separate PR |
|
I split the bugfixes to #1069 |
SebastianM-C
force-pushed
the
smc/madnlp
branch
from
90f20ed to
e9747e1
Compare
|
I rebased, I'll bump the OptimizationBase compat once we have a release |
This is a wrapper for https://github.com/MadNLP/MadNLP.jl Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
no support in MadNLP :(
Co-authored-by: Claude <noreply@anthropic.com>
Fixes dimension mismatch errors when using sparse Lagrangian Hessians & corrects constraint Jacobian structure initialization Co-authored-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Ensure Hessian buffers are created with numeric type T instead of inheriting Bool from sparsity prototypes. Fixes InexactError when storing Hessian values. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
SebastianM-C
force-pushed
the
smc/madnlp
branch
from
e9747e1 to
525fb64
Compare
|
Needs a follow up with a doc page, but since this is at least correct let's start the 3 day timer. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
Add wrapper for https://github.com/MadNLP/MadNLP.jl
Currently MadNLP does not support user defined callbacks :( I'll look into PRing that after this gets in.
While working on the tests I also found a couple of bugs in the AD subpackages and fixed them with some help from Claude Code. Should I PR the fixes separately? This passes the tests locally, but in CI it fails due to what I fixed in 90b5b1c
Speaking of bugs, I think the
SecondOrderwarning has a bug whenAutoSparseis used, but I'm not sure if I should address that now. I seeThis is currently WIP, it needs[ ] callback support[ ] progress supportInterface wise, I'm wondering if I should make the
linear_solverandkkt_systemmore easily accessible instead of being in the additional options. The reason for the current approach is that MadNLP picks the defaults for those based on the system and in our optimizer struct we don't have the system yet. One option would be to use some dummy value that is then later translated internally.