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

Improve GLPK error handling #19525

Closed
jdemeyer opened this issue Nov 4, 2015 · 48 comments
Closed

Improve GLPK error handling #19525

jdemeyer opened this issue Nov 4, 2015 · 48 comments

Comments

@jdemeyer
Copy link

jdemeyer commented Nov 4, 2015

This ticket adds a proper error handler for GLPK (similar to what we already have for PARI, NTL, ...). It suffices to add sig_on()/sig_off() around a GLPK call to get an error message.

Some more cleanup is also done:

  1. change the implementation of MIPSolverException to just inherit from RuntimeError without any custom implementation.
  2. use MemoryAllocator in a few places to make some allocations safer.

This branch requires a patch to GLPK, a modified version of it was accepted upstream: http://lists.gnu.org/archive/html/help-glpk/2015-11/msg00008.html

Upstream: Fixed upstream, in a later stable release.

CC: @nathanncohen

Component: cython

Author: Jeroen Demeyer

Branch/Commit: 76a11fd

Reviewer: Vincent Delecroix

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

@jdemeyer jdemeyer added this to the sage-6.10 milestone Nov 4, 2015
@jdemeyer
Copy link
Author

jdemeyer commented Nov 4, 2015

Dependencies: #19527

@jdemeyer
Copy link
Author

jdemeyer commented Nov 5, 2015

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 5, 2015

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

f808061Implement GLPK error handling

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 5, 2015

Commit: f808061

@jdemeyer
Copy link
Author

jdemeyer commented Nov 5, 2015

Upstream: Reported upstream. No feedback yet.

@jdemeyer

This comment has been minimized.

@jdemeyer

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 5, 2015

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

f312818Improve printing of solver exceptions

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 5, 2015

Changed commit from f808061 to f312818

@jdemeyer
Copy link
Author

jdemeyer commented Nov 5, 2015

Changed upstream from Reported upstream. No feedback yet. to Reported upstream. Developers acknowledge bug.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 6, 2015

Changed commit from f312818 to 18265ee

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 6, 2015

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

18265eeRename glp_have_error -> glp_at_error

@jdemeyer
Copy link
Author

jdemeyer commented Nov 6, 2015

Changed upstream from Reported upstream. Developers acknowledge bug. to Fixed upstream, but not in a stable release.

@jdemeyer
Copy link
Author

jdemeyer commented Nov 9, 2015

Changed upstream from Fixed upstream, but not in a stable release. to Fixed upstream, in a later stable release.

@jdemeyer
Copy link
Author

jdemeyer commented Nov 9, 2015

Changed upstream from Fixed upstream, in a later stable release. to Not yet reported upstream; Will do shortly.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

jdemeyer commented Nov 9, 2015

comment:14

Upstream claims we are mis-using the GLPK error hook. They are probably right. However, given that I'd like to keep "upgrade GLPK" separated from this ticket, I still propose to review this ticket as-is and then deal with the upgrade on a new ticket.

@jdemeyer
Copy link
Author

jdemeyer commented Nov 9, 2015

Changed upstream from Not yet reported upstream; Will do shortly. to Fixed upstream, in a later stable release.

@dimpase
Copy link
Member

dimpase commented Nov 11, 2015

comment:15

Does a reviewer need to create a patched GLPK tarball himself?

@jdemeyer
Copy link
Author

comment:16

Replying to @dimpase:

Does a reviewer need to create a patched GLPK tarball himself?

No. This isn't a GLPK upgrade.

@jdemeyer

This comment has been minimized.

@mkoeppe
Copy link
Member

mkoeppe commented Mar 30, 2016

comment:18

Any news on the upstream situation?

@videlec
Copy link
Contributor

videlec commented Mar 30, 2016

comment:19

does not apply on the latest beta

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 30, 2016

Changed commit from 5211376 to 3e33342

@videlec
Copy link
Contributor

videlec commented Mar 30, 2016

Changed branch from u/jdemeyer/improve_glpk_error_handling to u/vdelecroix/19525

@videlec
Copy link
Contributor

videlec commented Mar 30, 2016

Changed commit from 3e33342 to 71f52a4

@videlec
Copy link
Contributor

videlec commented Mar 30, 2016

comment:29

A commit to fix changes in error messages.


New commits:

71f52a4Trac 19525: fix error messages for cplex and gurobi

@mkoeppe
Copy link
Member

mkoeppe commented Mar 30, 2016

comment:30

See also #10232

@videlec
Copy link
Contributor

videlec commented Mar 30, 2016

comment:31

Indeed, in order to fix #10232 there need to be the appropriate sig_on and sig_off around the GLPK calls.

@videlec
Copy link
Contributor

videlec commented Mar 30, 2016

comment:32

Jeroen, do you prefer another ticket to add the appropriate sig_on/sig_off around GLPK calls?

@videlec
Copy link
Contributor

videlec commented Mar 30, 2016

comment:33

If yes, then the ticket is good to go from my point of view.

@jdemeyer
Copy link
Author

Reviewer: Vincent Delecroix

@vbraun
Copy link
Member

vbraun commented Mar 31, 2016

comment:35

Fails on 32-bit

sage -t --long src/sage/numerical/backends/glpk_backend.pyx
**********************************************************************
File "src/sage/numerical/backends/glpk_backend.pyx", line 847, in sage.numerical.backends.glpk_backend.GLPKBackend.solve
Failed example:
    lp.solve()
Expected:
    Traceback (most recent call last):
    ...
    GLPKError: Assertion failed: col->lb < col->ub
    Error detected in file glpnpp05.c at line ...
Got:
    <BLANKLINE>
    Traceback (most recent call last):
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 496, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 858, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.numerical.backends.glpk_backend.GLPKBackend.solve[24]>", line 1, in <module>
        lp.solve()
      File "sage/numerical/mip.pyx", line 2115, in sage.numerical.mip.MixedIntegerLinearProgram.solve (/home/buildbot/slave/sage_git/build/src/build/cythonized/sage/numerical/mip.c:14757)
        self._backend.solve()
      File "sage/numerical/backends/glpk_backend.pyx", line 1005, in sage.numerical.backends.glpk_backend.GLPKBackend.solve (/home/buildbot/slave/sage_git/build/src/build/cythonized/sage/numerical/backends/glpk_backend.cpp:8406)
        sig_on()
    GLPKError: Assertion failed: q->lb < q->ub
    Error detected in file glpnpp03.c at line 557
**********************************************************************
1 item had failures:
   1 of  76 in sage.numerical.backends.glpk_backend.GLPKBackend.solve

@jdemeyer
Copy link
Author

jdemeyer commented Apr 1, 2016

Changed branch from u/vdelecroix/19525 to u/jdemeyer/19525

@jdemeyer
Copy link
Author

jdemeyer commented Apr 1, 2016

comment:37

This should do the trick...


New commits:

76a11fdUse ellipsis in assertion error message

@jdemeyer
Copy link
Author

jdemeyer commented Apr 1, 2016

Changed commit from 71f52a4 to 76a11fd

@vbraun
Copy link
Member

vbraun commented Apr 1, 2016

Changed branch from u/jdemeyer/19525 to 76a11fd

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