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

py3: fix doctests with cbc #28689

Closed
dcoudert opened this issue Nov 2, 2019 · 21 comments
Closed

py3: fix doctests with cbc #28689

dcoudert opened this issue Nov 2, 2019 · 21 comments

Comments

@dcoudert
Copy link
Contributor

dcoudert commented Nov 2, 2019

The following doctests are failing with py3 and cbc, but not with py2. This is due to a different behavior of round called in get_variable_value.

Using --optional=benzene,bliss,build,cbc,csdp,dochtml,dot2tex,gap_packages,gfortran,igraph,libsemigroups,mcqd,mpir,plantri,python_igraph,sage,tdlib
Doctesting 1 file using 4 threads.
sage -t --long src/sage/numerical/backends/coin_backend.pyx
**********************************************************************
File "src/sage/numerical/backends/coin_backend.pyx", line 423, in sage.numerical.backends.coin_backend.CoinBackend.remove_constraint
Failed example:
    p.get_values([x,y])                          # optional - cbc
Expected:
    [0.0, 3.0]
Got:
    [0, 3]
**********************************************************************
File "src/sage/numerical/backends/coin_backend.pyx", line 461, in sage.numerical.backends.coin_backend.CoinBackend.remove_constraints
Failed example:
    p.get_values(x)                              # optional - cbc
Expected:
    2.0...
Got:
    2
**********************************************************************
File "src/sage/numerical/backends/coin_backend.pyx", line 463, in sage.numerical.backends.coin_backend.CoinBackend.remove_constraints
Failed example:
    p.get_values(y)                              # optional - cbc
Expected:
    0.0...
Got:
    0
**********************************************************************
File "src/sage/numerical/backends/coin_backend.pyx", line 468, in sage.numerical.backends.coin_backend.CoinBackend.remove_constraints
Failed example:
    p.get_values([x,y])                          # optional - cbc
Expected:
    [0.0, 3.0]
Got:
    [0, 3]
**********************************************************************
2 items had failures:
   1 of  13 in sage.numerical.backends.coin_backend.CoinBackend.remove_constraint
   3 of  15 in sage.numerical.backends.coin_backend.CoinBackend.remove_constraints
    [353 tests, 4 failures, 0.34 s]

Component: linear programming

Author: David Coudert

Branch/Commit: 0bbdca8

Reviewer: John Palmieri

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

@dcoudert dcoudert added this to the sage-9.0 milestone Nov 2, 2019
@dcoudert
Copy link
Contributor Author

dcoudert commented Nov 2, 2019

comment:1

The proposed solution unifies the behavior with cplex backend.


New commits:

997e76ftrac #28689: fix doctests with cbc

@dcoudert
Copy link
Contributor Author

dcoudert commented Nov 2, 2019

Commit: 997e76f

@dcoudert

This comment has been minimized.

@dcoudert
Copy link
Contributor Author

dcoudert commented Nov 2, 2019

Branch: public/linear_programming/28689_cbc

@jhpalmieri
Copy link
Member

comment:2

When I try to install cbc, I get an error:

mkdir: /Users/palmieri/Desktop/Sage_stuff/git/sage/local/var/tmp/sage/build/cbc-2.9.4.p0/inst/Users/palmieri/Desktop/Sage_stuff/git/sage/local/include/coin: File exists
make[5]: *** [install-includecoinHEADERS] Error 1
make[5]: *** Waiting for unfinished jobs....

I think it's a race condition. Please try building with MAKE="make -j4" (for example) to see if you run into this. For me, this fixes it:

diff --git a/build/pkgs/cbc/spkg-install b/build/pkgs/cbc/spkg-install
index 058c488ea7..d1b6446620 100644
--- a/build/pkgs/cbc/spkg-install
+++ b/build/pkgs/cbc/spkg-install
@@ -15,4 +15,4 @@ sed -i -e "s/clock\_gettime ()/Grrrrrrrrrrrr\ ()/g" Cbc/configure || \
 sdh_configure --enable-cbc-parallel --enable-parallel \
               --enable-gnu-packages --enable-static
 sdh_make
-sdh_make_install
+sdh_make_install -j1

@dcoudert
Copy link
Contributor Author

dcoudert commented Nov 2, 2019

comment:3

I don't understand what you are asking me to try, and note that I'm using a 5 years old macbook air with a dual core i7 1,7 GHz CPU...

@jhpalmieri
Copy link
Member

comment:4

Please set MAKE="make -j2" and MAKEFLAGS="-j2" (since your machine has 2 cores) and try to build cbc. I have these variables set to make sure I get a parallel build, and cbc fails for me; I'm asking if you see the same behavior. (It may be MAKEFLAGS rather than MAKE which is causing the problem, but the Sage installation guide suggests using either, so we should support setting either.)

If you do see this problem, then see if my proposed fix works.

Maybe other reviewers could do the same testing.

@dcoudert
Copy link
Contributor Author

dcoudert commented Nov 2, 2019

comment:5

I tried with -j2 and it's working (./sage -f cbc). However, with -j4 it's failing.

@jhpalmieri
Copy link
Member

comment:6

For me, too.

@dcoudert
Copy link
Contributor Author

dcoudert commented Nov 3, 2019

comment:7

feel free to push required changes here (branch in public)

@jhpalmieri
Copy link
Member

comment:8

I get a doctest failure:

sage -t --long --warn-long 64.7 src/sage/numerical/backends/gurobi_backend.pyx
**********************************************************************
File "src/sage/numerical/backends/gurobi_backend.pyx", line 13, in sage.numerical.backends.gurobi_backend
Failed example:
    g.feedback_edge_set(value_only = True, constraint_generation = False)
Exception raised:
    Traceback (most recent call last):
      File "/Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python3.7/site-packages/sage/doctest/forker.py", line 681, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python3.7/site-packages/sage/doctest/forker.py", line 1123, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.numerical.backends.gurobi_backend[1]>", line 1, in <module>
        g.feedback_edge_set(value_only = True, constraint_generation = False)
      File "/Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python3.7/site-packages/sage/graphs/digraph.py", line 1638, in feedback_edge_set
        value_only=True, solver=solver, verbose=verbose)
      File "/Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python3.7/site-packages/sage/graphs/digraph.py", line 1727, in feedback_edge_set
        return Integer(round(p.solve(objective_only=True, log=verbose)))
    TypeError: type sage.rings.real_double.RealDoubleElement doesn't define __round__ method
**********************************************************************
1 item had failures:
   1 of   3 in sage.numerical.backends.gurobi_backend
    [10 tests, 1 failure, 0.16 s]

I don't know what this has to do with cbc, but I get it when it's installed, I don't when it's not.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 3, 2019

Changed commit from 997e76f to 4310696

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 3, 2019

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

4310696trac 28689: do not install cbc in parallel to avoid race condition

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 4, 2019

Changed commit from 4310696 to 0bbdca8

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 4, 2019

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

323daa9trac #28689: Merged with 9.0.beta4
0bbdca8trac #28689: fix type issue with get_objective_value

@dcoudert
Copy link
Contributor Author

dcoudert commented Nov 4, 2019

comment:11

I don't have gurobi installed, but I have the same issue with coin (with and without this patch).

sage: G = digraphs.DeBruijn(2, 3)
sage: G.feedback_edge_set(value_only = True, constraint_generation = False, solver='coin')
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-3-1312c6b79adf> in <module>()
----> 1 G.feedback_edge_set(value_only = True, constraint_generation = False, solver='coin')

/Users/dcoudert/sage3/sage/local/lib/python3.7/site-packages/sage/graphs/digraph.py in feedback_edge_set(self, constraint_generation, value_only, solver, verbose)
   1621             D.allow_loops(False)
   1622             FAS = D.feedback_edge_set(constraint_generation=constraint_generation,
-> 1623                                           value_only=value_only, solver=solver, verbose=verbose)
   1624             if value_only:
   1625                 return FAS + self.number_of_loops()

/Users/dcoudert/sage3/sage/local/lib/python3.7/site-packages/sage/graphs/digraph.py in feedback_edge_set(self, constraint_generation, value_only, solver, verbose)
   1725 
   1726             if value_only:
-> 1727                 return Integer(round(p.solve(objective_only=True, log=verbose)))
   1728             else:
   1729                 p.solve(log=verbose)

TypeError: type sage.rings.real_double.RealDoubleElement doesn't define __round__ method
sage: G.feedback_edge_set(value_only = True, constraint_generation = False, solver='glpk')
6
sage: G.feedback_edge_set(value_only = True, constraint_generation = False, solver='cplex')
6

Applying the same receipe than for cplex and gurobi in method get_objective_value fix the issue for me.

@jhpalmieri
Copy link
Member

comment:12

I don't have gurobi installed either, by the way.

@dcoudert
Copy link
Contributor Author

dcoudert commented Nov 4, 2019

comment:13

The corresponding doctest in gurobi_backend.pyx is on purpose not restricted to a particular backend. So when you installed cbc / coin you highlighted an issue with this solver (it was your default). I have not seen it as I have cplex as default solver, and the issue has been fixed for cplex and gurobi in #27773. It should now be fixed for all solvers.

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@jhpalmieri
Copy link
Member

comment:14

Okay, looks good to me.

@vbraun
Copy link
Member

vbraun commented Nov 14, 2019

Changed branch from public/linear_programming/28689_cbc to 0bbdca8

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