Opened 4 years ago

Closed 13 months 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: sage-8.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) 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 19 months ago by mkoeppe

  • Cc dcoudert jmantysalo added
  • Dependencies set to #24782
  • Milestone changed from sage-7.2 to sage-8.2

comment:2 Changed 19 months ago by dcoudert

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.

comment:3 Changed 19 months ago by dcoudert

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

comment:4 Changed 19 months ago by dcoudert

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

comment:5 Changed 19 months ago by dcoudert

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

comment:6 Changed 19 months ago by dcoudert

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

The parameter solver was here but not used.

comment:7 follow-up: Changed 16 months ago by 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 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 16 months ago by dimpase

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.

comment:9 Changed 16 months ago by dcoudert

  • Authors set to David Coudert
  • Branch set to u/dcoudert/20416_parameter_solver
  • Commit set to ee6d1d75918b9f3cc577a3dea3f2daee002e5c53
  • Milestone changed from sage-8.2 to sage-8.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:

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

comment:10 Changed 15 months ago by vdelecroix

  • Milestone changed from sage-8.3 to sage-8.4

update milestone 8.3 -> 8.4

comment:11 Changed 14 months ago by git

  • Commit changed from ee6d1d75918b9f3cc577a3dea3f2daee002e5c53 to bb832ce9a2d9edd8fe16bd956fe6dc84b604d8ff

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

bb832cetrac #22050: Merged with 8.4.beta1

comment:12 Changed 13 months ago by git

  • Commit changed from bb832ce9a2d9edd8fe16bd956fe6dc84b604d8ff to 4e27e2ce65b3358b0f607fb2babc8bf08c50814a

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

4e27e2ctrac #20416: resolve merge conflicts with 8.4.beta4

comment:13 Changed 13 months ago by dcoudert

I fixed the conflicts with 8.4.beta4.

comment:14 Changed 13 months ago by dimpase

  • Dependencies #24782 deleted

comment:15 Changed 13 months ago by jmantysalo

  • 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 13 months ago by vbraun

  • Branch changed from u/dcoudert/20416_parameter_solver to 4e27e2ce65b3358b0f607fb2babc8bf08c50814a
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.