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

sage.graphs, sage.numerical.mip: Remove unnecessary uses of set_binary, set_integer #33504

Closed
mkoeppe opened this issue Mar 14, 2022 · 15 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Mar 14, 2022

We can avoid these methods that mutate variable type by setting up the correct variable type in the first place.

CC: @dcoudert

Component: linear programming

Author: Matthias Koeppe

Branch/Commit: b82a512

Reviewer: Travis Scrimshaw, David Coudert

Issue created by migration from https://trac.sagemath.org/ticket/33504

@mkoeppe mkoeppe added this to the sage-9.6 milestone Mar 14, 2022
@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 14, 2022

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 14, 2022

New commits:

cbfeef2MIPVariable: Avoid using _backend.set_variable_type
6c1a8f5vertex_separation_MILP: Remove redundant call of set_binary
44e219fGenericGraph.multicommodity_flow: Directly create MIP variables as integers, do not use set_integer

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 14, 2022

Author: Matthias Koeppe

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 14, 2022

Commit: 44e219f

@tscrim
Copy link
Collaborator

tscrim commented Mar 15, 2022

comment:3

LGTM. Green bot => positive review.

@tscrim
Copy link
Collaborator

tscrim commented Mar 15, 2022

Reviewer: Travis Scrimshaw

@dcoudert
Copy link
Contributor

comment:4

You did

-        flow = p.new_variable(nonnegative=True)
+        if integer:
+            flow = p.new_variable(nonnegative=True, integer=True)
+        else:
+            flow = p.new_variable(nonnegative=True)

but why not simply

-        flow = p.new_variable(nonnegative=True)
+        flow = p.new_variable(nonnegative=True, integer=integer is True)

@sheerluck
Copy link
Contributor

comment:5

Replying to @dcoudert:

flow = p.new_variable(nonnegative=True, integer=integer is True)
flow = p.new_variable(nonnegative=True, integer=integer)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 15, 2022

Changed commit from 44e219f to b82a512

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 15, 2022

Branch pushed to git repo; I updated commit sha1. New commits:

b82a512GenericGraph.multicommodity_flow: Simplify

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 15, 2022

comment:7

Thanks, done

@dcoudert
Copy link
Contributor

Changed reviewer from Travis Scrimshaw to Travis Scrimshaw, David Coudert

@dcoudert
Copy link
Contributor

comment:8

Green bot

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 16, 2022

comment:9

Thanks!

@vbraun
Copy link
Member

vbraun commented Mar 27, 2022

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

No branches or pull requests

5 participants