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_solver should allow passing a function (a solver factory) as the solver argument #20406

Closed
mkoeppe opened this issue Apr 9, 2016 · 26 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 9, 2016

For example, to use GLPK's exact simplex algorithm:

from sage.numerical.backends.generic_backend import get_solver
def glpk_exact_solver():
    b = get_solver(solver="GLPK")
    b.solver_parameter("simplex_or_intopt", "exact_simplex_only")
    return b

delsarte_bound_additive_hamming_space(19,15,7,solver=glpk_exact_solver)

CC: @dimpase @videlec @vbraun @jdemeyer

Component: numerical

Author: Matthias Koeppe

Branch/Commit: 37e87a5

Reviewer: Dima Pasechnik

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

@mkoeppe mkoeppe added this to the sage-7.2 milestone Apr 9, 2016
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 9, 2016

Branch: u/mkoeppe/get_solver_factory

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 9, 2016

Commit: b4146a7

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 9, 2016

New commits:

b4146a7get_solver: Allow a callable as the solver argument

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 9, 2016

Author: Matthias Koeppe

@mkoeppe

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 11, 2016

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

d9ce11eMixedIntegerLinearProgram.__copy__: Don't create a throw-away GLPK instance

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 11, 2016

Changed commit from b4146a7 to d9ce11e

@dimpase
Copy link
Member

dimpase commented Apr 13, 2016

comment:6

this also needs a rebase (sorry for sitting on this too long...)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 13, 2016

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

623a2e2get_solver: Allow a callable as the solver argument
09377acMixedIntegerLinearProgram.__copy__: Don't create a throw-away GLPK instance

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 13, 2016

Changed commit from d9ce11e to 09377ac

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 13, 2016

comment:8

Rebased on 7.2.beta4

@dimpase
Copy link
Member

dimpase commented Apr 14, 2016

comment:10

without the example in the ticket description it's hard to get the idea of this change.
Can you add this example into the code?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 14, 2016

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

d1d84c6get_solver: Add doctest
cc2d999Mention solver=callable in docstrings and error messages

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 14, 2016

Changed commit from 09377ac to cc2d999

@dimpase
Copy link
Member

dimpase commented Apr 14, 2016

comment:12

the doctest in the commit d1d84c6 appears to demonstrate a bug in the ILP solver of GLPK. The correct answer should be 3, not 2. Indeed, look at the entry n=19, k=2 in http://www.codetables.de/BKLC/Tables.php?q=7&n0=1&n1=100&k0=1&k1=100; it is 16, not 15.

(one also gets 3 when using PPL's ILP solver)

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 14, 2016

comment:13

GLPK has an exact simplex, not an exact MIP solver.
See #20446

@dimpase
Copy link
Member

dimpase commented Apr 14, 2016

comment:14

anyhow, this answer by GLPK is wrong, ILP or LP!
(the bound obtained by solving LP cannot be smaller than the one obtained from ILP)

sage: delsarte_bound_additive_hamming_space(19,15,7,isinteger=True) # ppl used
3
sage: delsarte_bound_additive_hamming_space(19,15,7) # ppl used
3

@dimpase
Copy link
Member

dimpase commented Apr 14, 2016

comment:15
sage: a,p,v=delsarte_bound_additive_hamming_space(19,15,7,solver=glpk_exact_solver,return_data=True)
...
sage:  [ZZ(j) for i,j in p.get_values(a).iteritems()]
[1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 48, 0, 0, 0, 0]

I need to do ZZ as otherwise it outputs floats. This is a maximization (of the sum of the coordinates) LP, and the following vector (computed by PPL) should be feasible, too:

[1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 307, 0, 0, 1, 34]

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 14, 2016

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

37e87a5Replace delsarte test by one that does not expose a bug in GLPK exact

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 14, 2016

Changed commit from cc2d999 to 37e87a5

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 14, 2016

comment:17

To move this ticket forward, I have replaced the test by something else

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 14, 2016

comment:18

The wrong result is now the subject of ticket #20447.

@dimpase
Copy link
Member

dimpase commented Apr 15, 2016

Reviewer: Dima Pasechnik

@dimpase
Copy link
Member

dimpase commented Apr 15, 2016

comment:19

OK!

@vbraun
Copy link
Member

vbraun commented Apr 15, 2016

Changed branch from u/mkoeppe/get_solver_factory to 37e87a5

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

3 participants