Opened 5 years ago
Closed 3 years ago
#20416 closed enhancement (fixed)
Various callers of MixedIntegerLinearProgram should accept and pass through a solver argument
Reported by:  mkoeppe  Owned by:  

Priority:  major  Milestone:  sage8.4 
Component:  numerical  Keywords:  lp 
Cc:  dimpase, vdelecroix, jdemeyer, dcoudert, jmantysalo  Merged in:  
Authors:  David Coudert  Reviewers:  Jori Mäntysalo 
Report Upstream:  N/A  Work issues:  
Branch:  4e27e2c (Commits, GitHub, GitLab)  Commit:  4e27e2ce65b3358b0f607fb2babc8bf08c50814a 
Dependencies:  Stopgaps: 
Description
src/sage/combinat/designs/orthogonal_arrays.py:1446: p = MixedIntegerLinearProgram() src/sage/combinat/designs/orthogonal_arrays_build_recursive.py:365: p = MixedIntegerLinearProgram() src/sage/combinat/integer_vector.py:361: p = MixedIntegerLinearProgram() src/sage/combinat/posets/posets.py:2730: p = MixedIntegerLinearProgram(constraint_generation=True) src/sage/geometry/cone.py:4328: p = MixedIntegerLinearProgram(maximization=False) src/sage/graphs/comparability.pyx:439: p = MixedIntegerLinearProgram() src/sage/graphs/digraph.py:1498: p = MixedIntegerLinearProgram(constraint_generation = True, src/sage/graphs/generic_graph.py:7137: p = MixedIntegerLinearProgram(maximization = False, src/sage/graphs/generic_graph.py:7589: p = MixedIntegerLinearProgram(constraint_generation = True, src/sage/matroids/matroid.pyx:7041: MIP = MixedIntegerLinearProgram(maximization=False) src/sage/numerical/optimize.py:801: p=MixedIntegerLinearProgram() src/sage/sat/solvers/sat_lp.py:33: self._LP = MixedIntegerLinearProgram()
Change History (16)
comment:1 Changed 3 years ago by
 Cc dcoudert jmantysalo added
 Dependencies set to #24782
 Milestone changed from sage7.2 to sage8.2
comment:2 Changed 3 years ago by
comment:3 Changed 3 years ago by
#24975 treat method binpacking
from src/sage/numerical/optimize.py
comment:4 Changed 3 years ago by
#24976 for several methods in src/sage/graphs/comparability.pyx
comment:5 Changed 3 years ago by
#24977 for method flat_cover
of src/sage/matroids/matroid.pyx
comment:6 Changed 3 years ago by
#24978 for class SatLP
of src/sage/sat/solvers/sat_lp.py
.
The parameter solver was here but not used.
comment:7 followup: ↓ 8 Changed 3 years ago by
We have added parameter solver
to all calls to MixedIntegerLinearProgram
. The last possible improvement is to allow to give the parameters solver
and verbose
to methods calling methods calling methods calling MixedIntegerLinearProgram
. For instance, method is_hamiltonian
calls traveling_salesman_problem
without allowing to give the parameters.
Do you think we should (try to) do it ? I don't know to trace calls to methods...
comment:8 in reply to: ↑ 7 Changed 3 years ago by
Replying to dcoudert:
We have added parameter
solver
to all calls toMixedIntegerLinearProgram
. The last possible improvement is to allow to give the parameterssolver
andverbose
to methods calling methods calling methods calling
no typo here? this looks scary...
MixedIntegerLinearProgram
. For instance, methodis_hamiltonian
callstraveling_salesman_problem
without allowing to give the parameters.Do you think we should (try to) do it ? I don't know to trace calls to methods...
one way would be to add print statements and then watch doctests failing...
Probably there are some static code analysis tools to help here too, but I'm no expert in these.
comment:9 Changed 3 years ago by
 Branch set to u/dcoudert/20416_parameter_solver
 Commit set to ee6d1d75918b9f3cc577a3dea3f2daee002e5c53
 Milestone changed from sage8.2 to sage8.3
 Status changed from new to needs_review
I added parameter solver to many methods in graphs/
. I have certainly missed some, but it's already an improvement. Not sure if I should do the same for other modules in this ticket. it's already a lot of modifications...
Last 10 new commits:
6c596d4  trac #20416: correct max_cut

2927b79  trac #20416: correct longest_path

ae072ef  trac #20416: add parameter solver to hamiltonian_cycle

c30b59a  trac #20416: improvement of flow

55dc77b  trac #20416: improvement of multicommodity_flow

46abf3e  trac #20416: improvement of disjoint_routed_paths

236abc1  trac #20416: add parameter solver to edge and vertex disjoint paths

15b0955  trac #20416: corrections in generic_graph.py

d5d8c6a  trac #20416: add parameter solver to methods in graph.py

ee6d1d7  trac #20416: add parameter solver to methods in graph_coloring.py

comment:10 Changed 3 years ago by
 Milestone changed from sage8.3 to sage8.4
update milestone 8.3 > 8.4
comment:11 Changed 3 years ago by
 Commit changed from ee6d1d75918b9f3cc577a3dea3f2daee002e5c53 to bb832ce9a2d9edd8fe16bd956fe6dc84b604d8ff
Branch pushed to git repo; I updated commit sha1. New commits:
bb832ce  trac #22050: Merged with 8.4.beta1

comment:12 Changed 3 years ago by
 Commit changed from bb832ce9a2d9edd8fe16bd956fe6dc84b604d8ff to 4e27e2ce65b3358b0f607fb2babc8bf08c50814a
Branch pushed to git repo; I updated commit sha1. New commits:
4e27e2c  trac #20416: resolve merge conflicts with 8.4.beta4

comment:13 Changed 3 years ago by
I fixed the conflicts with 8.4.beta4.
comment:14 Changed 3 years ago by
 Dependencies #24782 deleted
comment:15 Changed 3 years ago by
 Reviewers set to Jori Mäntysalo
 Status changed from needs_review to positive_review
Seems to be OK. Tests fail, but they are marked as # optional  magma
, so are not related to this patch.
comment:16 Changed 3 years ago by
 Branch changed from u/dcoudert/20416_parameter_solver to 4e27e2ce65b3358b0f607fb2babc8bf08c50814a
 Resolution set to fixed
 Status changed from positive_review to closed
It's certainly better to make different tickets to avoid conflicts since we will touch different modules. Furthermore, we can take the opportunity to improve some codes. For instance in
src/sage/numerical/optimize.py
, thebinpacking
method needs some polishing.