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

GenericBackend: Fix doctest of add_linear_constraint_vector #20326

Closed
mkoeppe opened this issue Mar 30, 2016 · 19 comments
Closed

GenericBackend: Fix doctest of add_linear_constraint_vector #20326

mkoeppe opened this issue Mar 30, 2016 · 19 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 30, 2016

The doctest of add_linear_constraint_vector from generic_backend.pyx (which is never run for any real backend!), when applied to COIN or GLPK leads to segfaults:

sage: coeffs = ([0, vector([1, 2])], [1, vector([2, 3])])
sage: upper = vector([5, 5])
sage: lower = vector([0, 0])
sage: from sage.numerical.backends.generic_backend import get_solver
sage: p = get_solver(solver = "Coin")      # optional - cbc
sage: p.add_linear_constraint_vector(2, coeffs, lower, upper, 'foo')
------------------------------------------------------------------------
0   signals.so                          0x0000000109df05c5 print_backtrace + 37
------------------------------------------------------------------------
Unhandled SIGSEGV: A segmentation fault occurred.
This probably occurred because a *compiled* module has a bug
in it and is not properly wrapped with sig_on(), sig_off().
Python will now terminate.
------------------------------------------------------------------------
Segmentation fault: 11
$ sage
SageMath Version 7.2.beta0, Release Date: 2016-03-24 
sage: coeffs = ([0, vector([1, 2])], [1, vector([2, 3])])
sage: upper = vector([5, 5])
sage: lower = vector([0, 0])
sage: from sage.numerical.backends.generic_backend import get_solver
sage: p = get_solver(solver = "Coin")       # optional - cbc
sage: p.add_linear_constraint_vector(2, coeffs, lower, upper)
------------------------------------------------------------------------
0   signals.so                          0x0000000109c8a5c5 print_backtrace + 37
------------------------------------------------------------------------
Unhandled SIGSEGV: A segmentation fault occurred.

CC: @dimpase @videlec

Component: numerical

Author: Matthias Koeppe

Branch/Commit: 690b1a5

Reviewer: Dima Pasechnik

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

@mkoeppe mkoeppe added this to the sage-7.2 milestone Mar 30, 2016
@dimpase

This comment has been minimized.

@dimpase
Copy link
Member

dimpase commented Apr 5, 2016

comment:1

with the second example I only get segfault with Coin - with other backends I get gracefully handled errors.
(on a branch with lots of recent positively reviewed LP-related tickets)

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 5, 2016

comment:2

The COIN segfaults are addressed in #20360.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 6, 2016

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 6, 2016

Commit: dea0705

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 6, 2016

New commits:

dea0705Fix #20326: GenericBackend: Fix doctest of add_linear_constraint_vector

@dimpase
Copy link
Member

dimpase commented Apr 6, 2016

comment:5

solver.add_variables(2) throws a NotImplementedError immediately, what's the point of the last commit then?

sage -t src/sage/numerical/backends/generic_backend.pyx
**********************************************************************
File "src/sage/numerical/backends/generic_backend.pyx", line 402, in sage.numerical.backends.generic_backend.GenericBackend.add_linear_constraint_vector
Failed example:
    solver.add_variables(2)
...
    NotImplementedError

@dimpase
Copy link
Member

dimpase commented Apr 6, 2016

comment:6

if you want to keep these as a template you'd mark them # not tested

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 6, 2016

Changed commit from dea0705 to b932b89

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 6, 2016

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

b932b89add_linear_constraint_vector: Fix fix of doctest

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 6, 2016

comment:8

Sorry! I tested the wrong file after making this change, so I did not catch this test failure.

Yes, the idea is to make generic_backend.pyx suitable for cut and paste again.
But don't worry, I won't make many more tickets like this one; I have been making progress with #20323.

@dimpase
Copy link
Member

dimpase commented Apr 6, 2016

comment:9

Isn't it more in line with the rest of the code to tag these statements by # optional - Nonexistent_LP_solver ? Then you don't need to insert Tracebacks...

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 6, 2016

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

b3a14bbFix #20326: GenericBackend: Fix doctest of add_linear_constraint_vector
690b1a5add_linear_constraint: Use 'optional - Nonexistent_LP_solver' test as well

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 6, 2016

Changed commit from b932b89 to 690b1a5

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 6, 2016

comment:11

Replying to @dimpase:

Isn't it more in line with the rest of the code to tag these statements by # optional - Nonexistent_LP_solver ? Then you don't need to insert Tracebacks...

You are right. I have now brought it in line with the doctests for the other methods. Same also for add_linear_constraint.

@dimpase
Copy link
Member

dimpase commented Apr 6, 2016

comment:12

OK, good!

@dimpase
Copy link
Member

dimpase commented Apr 6, 2016

Author: Matthias Koeppe

@dimpase
Copy link
Member

dimpase commented Apr 6, 2016

Reviewer: Dima Pasechnik

@vbraun
Copy link
Member

vbraun commented Apr 8, 2016

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