Skip to content

Fix vector functions#345

Open
joaquimg wants to merge 10 commits into
masterfrom
jg/fix-vec
Open

Fix vector functions#345
joaquimg wants to merge 10 commits into
masterfrom
jg/fix-vec

Conversation

@joaquimg
Copy link
Copy Markdown
Member

Most important to agree on are the functions on jump_wrapper.jl
Later on, we will need to deprecate the MOI functions that are not compliant with #288

Claude helped me with the bridging stuff and created first version of tests.

close #103
close #283
close #288
close #305

@joaquimg joaquimg mentioned this pull request Mar 23, 2026
Comment thread src/bridges.jl
Comment thread src/jump_wrapper.jl
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 23, 2026

Codecov Report

❌ Patch coverage is 96.27329% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.16%. Comparing base (e395b28) to head (9eb811b).
⚠️ Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
src/bridges.jl 91.83% 4 Missing ⚠️
src/moi_wrapper.jl 66.66% 1 Missing ⚠️
src/parameters.jl 98.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #345      +/-   ##
==========================================
+ Coverage   90.73%   91.16%   +0.42%     
==========================================
  Files          16       16              
  Lines        2300     2457     +157     
==========================================
+ Hits         2087     2240     +153     
- Misses        213      217       +4     

☔ 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.

Comment thread src/jump_wrapper.jl Outdated
Comment thread src/bridges.jl Outdated
Comment thread src/bridges.jl
@joaquimg
Copy link
Copy Markdown
Member Author

joaquimg commented Mar 24, 2026

Open question:
Do we keep model as an input argument to set_forward_constraint_function and others?
Or we just get it through owner_model?

@joaquimg joaquimg requested a review from blegat March 25, 2026 03:14
@joaquimg
Copy link
Copy Markdown
Member Author

@odow do you want to comment on the proposed functions in jump_wrapper.jl?

Comment thread src/jump_wrapper.jl
ForwardObjectiveFunction(),
JuMP.moi_function(func),
)
end
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.

The other option for this is

set_attribute(model, DiffOpt.ForwardObjectiveFunction(), func)

It requires fewer new functions.

Comment thread src/jump_wrapper.jl
JuMP.index(con_ref),
JuMP.moi_function(func),
)
end
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.

This one would be

set_attribute(con_ref, ForwardConstraintFunction(), func)

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.

We tend not to require both model and con_ref when model = owner_model(con_ref)

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.

Yeah, I noticed this after finishing.
I also think I prefer without the model.

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

Labels

None yet

3 participants