search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.43k stars 479 forks source link

Various callers of MixedIntegerLinearProgram should accept and pass through a solver argument #20416

Closed mkoeppe closed 6 years ago

mkoeppe commented 8 years ago 
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()

CC: @dimpase @videlec @jdemeyer @dcoudert @jm58660

Component: numerical

Author: David Coudert

Branch/Commit: 4e27e2c

Reviewer: Jori Mäntysalo

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

mkoeppe commented 6 years ago

Dependencies: #24782

dcoudert commented 6 years ago
comment:2

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, the binpacking method needs some polishing.

dcoudert commented 6 years ago
comment:3

24975 treat method binpacking from src/sage/numerical/optimize.py

dcoudert commented 6 years ago
comment:4

24976 for several methods in src/sage/graphs/comparability.pyx

dcoudert commented 6 years ago
comment:5

24977 for method flat_cover of src/sage/matroids/matroid.pyx

dcoudert commented 6 years ago
comment:6

24978 for class SatLP of src/sage/sat/solvers/sat_lp.py.

The parameter solver was here but not used.

dcoudert commented 6 years ago
comment:7

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

dimpase commented 6 years ago
comment:8

Replying to @dcoudert:

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

no typo here? this looks scary...

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

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.

dcoudert commented 6 years ago

Author: David Coudert

dcoudert commented 6 years ago

Branch: u/dcoudert/20416_parameter_solver

dcoudert commented 6 years ago
comment:9

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:

6c596d4trac #20416: correct max_cut
2927b79trac #20416: correct longest_path
ae072eftrac #20416: add parameter solver to hamiltonian_cycle
c30b59atrac #20416: improvement of flow
55dc77btrac #20416: improvement of multicommodity_flow
46abf3etrac #20416: improvement of disjoint_routed_paths
236abc1trac #20416: add parameter solver to edge and vertex disjoint paths
15b0955trac #20416: corrections in generic_graph.py
d5d8c6atrac #20416: add parameter solver to methods in graph.py
ee6d1d7trac #20416: add parameter solver to methods in graph_coloring.py
dcoudert commented 6 years ago

Commit: ee6d1d7

videlec commented 6 years ago
comment:10

update milestone 8.3 -> 8.4

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 6 years ago

Changed commit from ee6d1d7 to bb832ce

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 6 years ago

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

bb832cetrac #22050: Merged with 8.4.beta1
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 6 years ago

Changed commit from bb832ce to 4e27e2c

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 6 years ago

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

4e27e2ctrac #20416: resolve merge conflicts with 8.4.beta4
dcoudert commented 6 years ago
comment:13

I fixed the conflicts with 8.4.beta4.

dimpase commented 6 years ago

Changed dependencies from #24782 to none

f29946bc-ee7b-48cd-9abc-3445948c551d commented 6 years ago
comment:15

Seems to be OK. Tests fail, but they are marked as # optional - magma, so are not related to this patch.

f29946bc-ee7b-48cd-9abc-3445948c551d commented 6 years ago

Reviewer: Jori Mäntysalo

vbraun commented 6 years ago

Changed branch from u/dcoudert/20416_parameter_solver to 4e27e2c