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

1 doctest failing in combinat/designs/incidence_structures.py #32053

Open
seblabbe opened this issue Jun 24, 2021 · 21 comments
Open

1 doctest failing in combinat/designs/incidence_structures.py #32053

seblabbe opened this issue Jun 24, 2021 · 21 comments

Comments

@seblabbe
Copy link
Contributor

seblabbe commented Jun 24, 2021

On Ubuntu 18.04 with 9.4.beta3 with the following installed optional packages:

Using --optional=4ti2,build,ccache,cryptominisat,debian,dochtml,dot2tex,
e_antic,fricas,glucose,latte_int,lidia,normaliz,notedown,
pandoc_attributes,pip,pycosat,pynormaliz,rst2ipynb,sage,
sage_numerical_backends_coin,sage_spkg

the command

sage -t src/sage/combinat/designs/incidence_structures.py

gives

sage -t --random-seed=0 src/sage/combinat/designs/incidence_structures.py
**********************************************************************
File "src/sage/combinat/designs/incidence_structures.py", line 2027, in sage.combinat.designs.incidence_structures.IncidenceStructure.coloring
Failed example:
    len(designs.steiner_triple_system(7).coloring())
Expected:
    3
Got:
    2
**********************************************************************
1 item had failures:
   1 of   6 in sage.combinat.designs.incidence_structures.IncidenceStructure.coloring
    [338 tests, 1 failure, 0.93 s]
----------------------------------------------------------------------
sage -t --random-seed=0 src/sage/combinat/designs/incidence_structures.py  # 1 doctest failed
----------------------------------------------------------------------

CC: @mkoeppe @slel

Component: doctest coverage

Keywords: coin

Branch/Commit: u/mkoeppe/1_doctest_failing_in_combinat_designs_incidence_structures_py @ b07d203

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

@seblabbe seblabbe added this to the sage-9.4 milestone Jun 24, 2021
@slel
Copy link
Member

slel commented Jul 26, 2021

comment:1

The coloring method of Steiner triple systems
calls MixedIntegerLinearProgram, which takes
a solver argument, defaulting to None, in
which case default_mip_solver is used.

Testing on two different Sage installations:

sage: default_mip_solver()
'Glpk'
sage: len(designs.steiner_triple_system(7).coloring())
3
sage: default_mip_solver()
'Coin'
sage: len(designs.steiner_triple_system(7).coloring())
2

A previous report was at #30637.

@slel
Copy link
Member

slel commented Jul 26, 2021

Changed keywords from none to coin

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 26, 2021

comment:2

This will need testing in the next beta, which has many fixes for the use of MIP (see Meta-ticket #32191)

@sheerluck
Copy link
Contributor

comment:3

bug is in line 2048 of src/sage/combinat/designs/incidence_structures.py

return self.coloring(k)

Recursive call loses value of solver:

sage: designs.steiner_triple_system(7).coloring(solver="literally whatever")
[[1, 2, 3, 4], [0, 5, 6]]

fix would be

return self.coloring(k, solver=solver)

That wolud lead to

sage: designs.steiner_triple_system(7).coloring(solver="GLPK")
[[0, 1], [3], [2, 4, 5, 6]]
sage: designs.steiner_triple_system(7).coloring(solver="Coin")
[[1, 2, 3, 4], [0, 5, 6]]
sage: designs.steiner_triple_system(7).coloring(solver="PPL")
[[0, 1, 2, 5], [3], [4, 6]]

@sheerluck
Copy link
Contributor

comment:4

Even better:

return self.coloring(k, solver=solver, verbose=verbose)

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 26, 2021

Dependencies: #32218

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 26, 2021

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 26, 2021

Commit: b07d203

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 26, 2021

comment:7

Did this on top of #32218.


Last 10 new commits:

5a74113MixedIntegerLinearProgram._backend_variable_value: Add docstring
aa4eed5MixedIntegerLinearProgram._backend_variable_value*: Add docstrings, examples
00b34aatrac #32197: review commit
db83853trac #32197: another review commit
2038cc7BalancedIncompleteBlockDesign.arc: Use get_values(..., convert=bool, tolerance=integrality_tolerance)
7ef58e9sage.combinat.designs: Fix remaining uses of MixedIntegerLinearProgram.get_values
5af993ftrac #32218: review commit
66c5918Merge #32218
c77191bIncidenceStructure.coloring: Change doctest from # not tested to # random
b07d203IncidenceStructure.coloring: In self-call, pass on parameters

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 26, 2021

comment:8

I cannot reproduce this behavior - which is clearly a bug

sage: designs.steiner_triple_system(7).coloring(solver="Coin")
[[1, 2, 3, 4], [0, 5, 6]]

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 26, 2021

comment:9

With which version of CBC do you get this result? We observed similar trouble in #30635

@sheerluck
Copy link
Contributor

comment:10

master branch of https://github.com/coin-or/Cbc with --enable-cbc-parallel because coinor-cbc-2.10.5.ebuild in Gentoo does not have --enable-cbc-parallel and that leads to failed assert(masterThread_); in CbcModel::doOneNode

@sheerluck
Copy link
Contributor

comment:11

I mean stable/2.10 not master. It is 2.10.5 + 43 commits

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 26, 2021

comment:12

Yes, I also saw these problems in #30635 with the upgrade to cbc 2.10.5 in #30644.

I think the coin interface (https://github.com/sagemath/sage-numerical-backends-coin) needs to be fixed. See #30635 comment:41 for a possible fix.

I won't have time to work on this before mid August unfortunately.

@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Aug 22, 2021
@sheerluck
Copy link
Contributor

comment:14

I noticed some activity at https://github.com/coin-or/Cbc.git then I rebuilded [Cbc, Cgl, Clp, CoinUtils, Osi] from master, saw same behavior for solver="Coin" and decided to ping here (I failed to fix sage-numerical-backends-coin)

@fchapoton
Copy link
Contributor

comment:15

bump to 9.6

@fchapoton fchapoton modified the milestones: sage-9.5, sage-9.6 Jan 29, 2022
@sheerluck
Copy link
Contributor

comment:16

for current master branches of coinor-utils coinor-osi coinor-cgl coinor-clp coinor-cbc one can build sage_numerical_backends_coin only without "CbcSolver" in coin_backend.pyx:

--- a/sage_numerical_backends_coin/coin_backend.pyx
+++ b/sage_numerical_backends_coin/coin_backend.pyx
@@ -1,5 +1,5 @@
 # distutils: language = c++
-# distutils: libraries = Cbc CbcSolver Cgl Clp CoinUtils OsiCbc OsiClp Osi
+# distutils: libraries = Cbc Cgl Clp CoinUtils OsiCbc OsiClp Osi
 """
 COIN Backend
 """

@sheerluck
Copy link
Contributor

comment:17

I was hoping to use #31962 this way:

from sage.numerical.backends.generic_sdp_backend import get_solver
designs.steiner_triple_system(7).coloring(solver=lambda: get_solver("cvxpy/CBC"))

but I got

TypeError: Cannot convert CVXPYSDPBackend to sage.numerical.backends.generic_backend.GenericBackend

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 9, 2022

comment:18

So far #31962 only provides an (unfinished) SDP backend, not an LP/MIP backend. This would have to be written separately. Any help is welcome on #31962!

@sheerluck
Copy link
Contributor

comment:19

when #31962 is merged and closed, "Coin" should not have a chance to be default_mip_solver.

Only if "Coin" is replaced with "cvxpy/CBC" then len(designs.steiner_triple_system(7).coloring()) will be 3 on every Sage installations again.

@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 May 3, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Sep 19, 2022
@mkoeppe mkoeppe removed this from the sage-9.8 milestone Jan 29, 2023
@sheerluck
Copy link
Contributor

I have a feeling this issue is solved in master branch

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