Skip to content

Conversation

@HunkBenny
Copy link
Contributor

Hi!

I am not certain about this one;

If C returns SCIP_INVALID, I think this method should return None instead.

SCIPgetVarRedcost can return SCIP_INVALID as per the docs. I believe that I once observed this to be equal to 1e100 in Rust (instead of the declared 1e99). I could not find SCIP_INVALID in scip-sys, is this something I should add there first?

So, for the moment, maybe do not merge this. Just looking for a second opinion

@HunkBenny
Copy link
Contributor Author

I made a mistake in 'redcost' I believe. I'll fix it in a bit

Now this method is called in the `redcost`-method of the `Variable`-struct.
Now, we do not check if the variable is in the LP anymore. Instead, we simply check if the variable has a column associated with it. This is because newly generated columns will not be in the LP, however they are columns and can have reduced cost associated with them.
@codecov
Copy link

codecov bot commented Nov 29, 2025

Codecov Report

❌ Patch coverage is 83.72093% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.81%. Comparing base (844223e) to head (f4e12e7).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/variable.rs 82.93% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #269      +/-   ##
==========================================
+ Coverage   76.49%   76.81%   +0.32%     
==========================================
  Files          29       29              
  Lines        3871     3915      +44     
==========================================
+ Hits         2961     3007      +46     
+ Misses        910      908       -2     

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

@HunkBenny
Copy link
Contributor Author

I saw the code cov report, neat tool. I added some extra lines to my test because of it. I did not manage to cover the path when a variable is "Loose" or "Fixed" though.

Let me know if you think it is not okay to merge! 😄

@mmghannam mmghannam changed the title Added get_redcost method to Variable.rs Added redcost method to Variable.rs Dec 16, 2025
@mmghannam mmghannam merged commit 2622e5e into scipopt:main Dec 16, 2025
8 checks passed
@HunkBenny HunkBenny deleted the variable branch December 16, 2025 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants