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

CoinBackend: _test_solve fails on 32-bit #21449

Closed
jdemeyer opened this issue Sep 8, 2016 · 29 comments
Closed

CoinBackend: _test_solve fails on 32-bit #21449

jdemeyer opened this issue Sep 8, 2016 · 29 comments

Comments

@jdemeyer
Copy link

jdemeyer commented Sep 8, 2016

On Linux arando 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:05:16 UTC 2016 i686 i686 i686 GNU/Linux:

**********************************************************************
File "src/sage/numerical/backends/coin_backend.pyx", line 39, in sage.numerical.backends.coin_backend.CoinBackend
Failed example:
    TestSuite(p).run(skip="_test_pickling")               # optional - cbc
Expected nothing
Got:
    Failure in _test_solve:
    Traceback (most recent call last):
      File "/home/jdemeyer/sage-git/local/lib/python2.7/site-packages/sage/misc/sage_unittest.py", line 283, in run
        test_method(tester = tester)
      File "sage/numerical/backends/generic_backend.pyx", line 697, in sage.numerical.backends.generic_backend.GenericBackend._test_solve (build/cythonized/sage/numerical/backends/generic_backend.c:8748)
        with tester.assertRaises(MIPSolverException) as cm:   # unbounded
      File "/home/jdemeyer/sage-git/local/lib/python/unittest/case.py", line 116, in __exit__
        "{0} not raised".format(exc_name))
    AssertionError: MIPSolverException not raised
    ------------------------------------------------------------
    The following tests failed: _test_solve
**********************************************************************

This is what is happening on a 32-bit Linux:

sage: from sage.numerical.backends.coin_backend import CoinBackend
sage: p = CoinBackend()
sage: p.add_linear_constraints(5, 0, None)
sage: p.add_col(range(5), range(5))
sage: p.objective_coefficient(0,1)
sage: p.solve()
0
sage: p.set_verbosity(3)
sage: p.solve()
Cbc3007W No integer variables - nothing to do
Clp0006I 0  Obj 0 Dual inf 0.9999999 (1)
Clp0000I Optimal - objective value -1.7976931e+308
0   ### Instead, an exception should have been raised.
sage: p.get_variable_value(0)
1.7976931348623157e+308  ### BAD

CC: @mkoeppe

Component: doctest coverage

Keywords: sdl

Author: Jeroen Demeyer

Branch: 5eacf72

Reviewer: Thierry Monteil, Matthias Koeppe

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

@jdemeyer jdemeyer added this to the sage-7.4 milestone Sep 8, 2016
@jdemeyer
Copy link
Author

jdemeyer commented Sep 8, 2016

@jdemeyer
Copy link
Author

jdemeyer commented Sep 8, 2016

New commits:

4b1e5a7CoinBackend: _test_solve fails on 32-bit

@jdemeyer
Copy link
Author

jdemeyer commented Sep 8, 2016

Commit: 4b1e5a7

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 8, 2016

Changed commit from 4b1e5a7 to 33ad25e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 8, 2016

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

33ad25eCoinBackend: _test_solve fails on 32-bit

@mkoeppe
Copy link
Member

mkoeppe commented Sep 8, 2016

comment:4

"known bug on 32 bit" -- to whom is this bug known?

@jdemeyer
Copy link
Author

jdemeyer commented Sep 8, 2016

comment:5

Replying to @mkoeppe:

"known bug on 32 bit" -- to whom is this bug known?

To you now :-)

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Sep 8, 2016

comment:6

I also fall on this problem. I see that the fix is to ignore the doctest for 32bit systems. How do you know that the doctest failure does not mean that there is a problem somewhere (that should be addressed) ?

@mkoeppe
Copy link
Member

mkoeppe commented Sep 8, 2016

comment:7

There definitely is a problem, and it should NOT be ignored.

@jdemeyer
Copy link
Author

jdemeyer commented Sep 9, 2016

comment:8

Replying to @sagetrac-tmonteil:

How do you know that the doctest failure does not mean that there is a problem somewhere

I am not claiming that. I am adding # known bug, which means that I agree that there is a problem somewhere.

Of course, if the real bug can be fixed, that would be better. But in the mean time, just to have all doctests formally passing (and make a 32-bit patchbot useful), I propose to add the # known bug.

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Sep 13, 2016

comment:9

If Mathias is OK with that, i am also in favor to set this ticket to positive review (i confirm that the patch fixes the doctest on my 32bit VM).

If someone has an idea where the bug comes from and how to fix it, that would be great.

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Sep 13, 2016

Reviewer: Thierry Monteil

@mkoeppe
Copy link
Member

mkoeppe commented Sep 13, 2016

comment:10

I am building a 32-bit environment at the moment to look at what's going on here.

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Member

mkoeppe commented Sep 15, 2016

comment:12

Upgrading CBC to 2.9.8 (latest) does NOT fix this problem.

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Sep 16, 2016

comment:13

Replying to @mkoeppe:

Upgrading CBC to 2.9.8 (latest) does NOT fix this problem.

Do you have an idea about where the problem comes from ? Does upstream say something about this ?

@mkoeppe
Copy link
Member

mkoeppe commented Sep 16, 2016

comment:14

I haven't checked with upstream yet.
I am actually not sure yet if this a problem with the library or with our Cython wrapper.

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Sep 20, 2016

comment:15

Replying to @mkoeppe:

I haven't checked with upstream yet.
I am actually not sure yet if this a problem with the library or with our Cython wrapper.

So shall we accept the current fix and postpone a real fix for when we understant more the situation ?

@mkoeppe
Copy link
Member

mkoeppe commented Sep 20, 2016

comment:16

The patch at least needs to include reference to a ticket; and when this ticket is closed a follow-up tickets for an actual fix needs to be opened.

@jdemeyer
Copy link
Author

comment:17

I created a new ticket at #21550 (details to be filled in) and added a reference to that ticket in the doctest.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 21, 2016

Changed commit from 33ad25e to 5eacf72

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 21, 2016

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

5eacf72Add ticket number

@dimpase
Copy link
Member

dimpase commented Sep 21, 2016

comment:19

I am surprised to see that sys.float_info returns the same on 32-bit and on 64-bit Linux. On 32-bit:

dima@arando:~$ python
Python 2.7.6 (default, Jun 22 2015, 18:00:18) 
[GCC 4.8.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.float_info
sys.float_info(max=1.7976931348623157e+308, max_exp=1024, max_10_exp=308, min=2.2250738585072014e-308, min_exp=-1021, min_10_exp=-307, dig=15, mant_dig=53, epsilon=2.220446049250313e-16, radix=2, rounds=1)

anyhow we might be hitting a classic like https://gcc.gnu.org/bugzilla/show_bug.cgi?id=323

@jdemeyer
Copy link
Author

comment:20

Replying to @dimpase:

I am surprised to see that sys.float_info returns the same on 32-bit and on 64-bit Linux. On 32-bit:

Why? The C type double is the same on 32-bit and 64-bit architectures.

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Sep 21, 2016

Changed reviewer from Thierry Monteil to Thierry Monteil, ​Matthias Koeppe

@vbraun
Copy link
Member

vbraun commented Sep 24, 2016

Changed branch from u/jdemeyer/coinbackend___test_solve_fails_on_32_bit to 5eacf72

@jdemeyer
Copy link
Author

Changed reviewer from Thierry Monteil, ​Matthias Koeppe to Thierry Monteil, Matthias Koeppe

@jdemeyer
Copy link
Author

Changed commit from 5eacf72 to none

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Aug 27, 2019

Changed keywords from none to sdl

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

4 participants