Skip to content
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

Get correct primal for MIP problems [needs review] #23

Merged
merged 2 commits into from
Feb 22, 2017

Conversation

cdiener
Copy link
Member

@cdiener cdiener commented Feb 20, 2017

Pinging @KristianJensen for review.

Tried to do it like optlang but realized that optlang has a bug when getting primals from MIP problems. If the problem is a MIP problem all primal values have to be requested with glp_mip_col_val including continuous columns. Otherwise you will get incorrect primals for columns that are continuous but constrained by integer variables since it requests them from the "normal" LP solution that ignores integer variables.

For now get_col_primals will deem a problem a MIP problem if there is at least one binary or integer variable and get all primals from the MIP solution in that case.

@cdiener cdiener changed the title Get correct primal for MIP problems Get correct primal for MIP problems [needs review] Feb 20, 2017
@KristianJensen
Copy link

This looks good. Nice catch on the optlang bug!

@KristianJensen
Copy link

KristianJensen commented Feb 21, 2017

Actually, I guess the get_row_primals should also use the glp_mip_row_val method if the problem is MIP?

@cdiener
Copy link
Member Author

cdiener commented Feb 21, 2017

Yes, good catch! I don't really know what do about the duals. There don't seem to be functions to get duals from MIP problems in GLPK....

@KristianJensen
Copy link

From a bit of google searching, I don't think dual values are well defined for MIP problems, or at least not calculated with the normal optimization algorithms. It seems most solvers just return the dual from the LP relaxation instead, so doing that here as well should be fine.

swiglpk/glpk.i Outdated
for(i=1; i<=n; i++){
prim = glp_get_col_prim(P, i);
if (n_int == 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Since speed is a concern, wouldn't it be better to perform the conditional check outside the loop? Or is this anyway optimized by the compiler?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch! I don't know if it does but better to do it explicitly 👍

@cdiener
Copy link
Member Author

cdiener commented Feb 21, 2017

@KristianJensen ok, added it as suggested.

@Midnighter Did make the change but it didn't affect performance at all. I suppose the compiler is good enough in branch optimization.

@KristianJensen KristianJensen merged commit e7f9459 into opencobra:master Feb 22, 2017
@cdiener cdiener deleted the fix/mip_primals branch February 22, 2017 17:38
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.

None yet

3 participants