Skip to content

Interface for solver supporting diff natively#356

Open
blegat wants to merge 17 commits intomasterfrom
bl/native
Open

Interface for solver supporting diff natively#356
blegat wants to merge 17 commits intomasterfrom
bl/native

Conversation

@blegat
Copy link
Copy Markdown
Member

@blegat blegat commented Mar 25, 2026

First commit by Claude Code during our meeting based on this prompt #344 (comment)
Then I took control

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 26, 2026

Codecov Report

❌ Patch coverage is 92.94118% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.00%. Comparing base (2b4c350) to head (c49c519).

Files with missing lines Patch % Lines
src/moi_wrapper.jl 89.28% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #356      +/-   ##
==========================================
+ Coverage   90.73%   91.00%   +0.26%     
==========================================
  Files          16       16              
  Lines        2300     2378      +78     
==========================================
+ Hits         2087     2164      +77     
- Misses        213      214       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@PTNobel
Copy link
Copy Markdown

PTNobel commented Mar 27, 2026

So I built a draft implementation for Moreau.jl using this branch; it worked really well! Thank you.

A few issues that popped up:

  • We need to decide whether to cache data for the backward pass when we solve the forward problem. There doesn't seem to be any way to know if we're being called from a DiffOpt context or not, so I ended up using the presence of the DiffOpt library as a proxy for whether to store backward gradients.
  • We support differentiating the quadratic term in the objective, and it seemed to me DiffOpt only supported linear sensitivity of the obj? I wasn't sure.
  • Converting from dobj (DiffOpt) to dx (Moreau interface) required us to cache a bunch of data structures like P, x, and q that we otherwise didn't need in the Julia interface.
  • We ended up needing to cache the structure of A on the Julia side to dA and db into dconstraint

Overall, I think a bunch of this is just API differences, and probably can't be fixed at this point, but figured it was worth mentioning.

#
# min (1/2) x'Qx + c'x
# s.t. Ax = b
# ─────────────────────────────────────────────────────────────────────────────
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Did you copy this from #344?

https://github.com/jump-dev/DiffOpt.jl/pull/344/changes#diff-5c4db019bd25c07f7ae70b995aa20e622fd98ef5ba02b0d7b7b8a36307e9a742

Otherwise, this raises a lot of questions from a copyright perspective that I don't know we're well prepared to handle.

In this case it's okay. But how do we have any control that the code is coming from a JuMP-dev repo and not some external source? And even it if is from a JuMP repo, what if the license is not MIT? And how can we ensure we keep any appropriate copyright etc?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There must be a tool for searching online and verifying.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure, but do I need to do that for every PR? When someone manually submits a PR we trust them that they haven't copied someone (without declaring it). I don't have that trust if it's Claude.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Cant we run the auto check in CI?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What auto check?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh you mean if there is such a tool, then we could run it in CI. The problem is: which tool?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, it was copied from there. I agree it is a concern

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You copied or Claude copied?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants