-
-
Notifications
You must be signed in to change notification settings - Fork 487
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
doctest failures related to automorphism groups of edge-labelled graphs #25399
Comments
comment:3
My own testing seems to point to |
comment:4
It's probably
robust tests should check that these groups are the same; isomorphism might be too slow, With orbits, it's even more obvious---different generator sets lead to different ordering of elements in orbits, so they should be compared as sets. |
comment:5
This appears to be a consequence of this change diff --git a/src/sage/graphs/generic_graph.py b/src/sage/graphs/generic_graph.py
index eea21a2d2e..49eadaf162 100644
--- a/src/sage/graphs/generic_graph.py
+++ b/src/sage/graphs/generic_graph.py
@@ -21923,7 +21923,10 @@ class GenericGraph(GenericGraph_pyx):
if (algorithm == 'bliss' or # explicit choice from the user; or
(algorithm is None and # by default
- not edge_labels and
have_bliss)):
- if edge_labels:
- raise ValueError("bliss cannot be used when edge_labels is True")
Bliss().require() from #24924. Reverting it fixes all the failing doctests. |
comment:6
this change is correct, it introduces functionality that allows bliss to be used on graphs with edge labels. The doctests should be fixed, obviously. why we did not see these test failures while working on #24924, I don't know. Perhaps an oversight on our side. |
Author: Dima Pasechnik |
Branch: u/dimpase/blissgeom |
Commit: |
comment:8
this fixes all but New commits:
|
comment:10
namely, it boils down to bliss failing on
|
comment:11
OK, the branch fixes those 4 doctests for me. So I am OK with that part. |
This comment has been minimized.
This comment has been minimized.
Reviewer: François Bissey |
comment:14
LGTM |
comment:15
polyhedron/base.py fails tests |
comment:16
I am guessing we overlooked testing without |
comment:17
OK so without
|
comment:18
Note: I created yesterday #25475 which turned out to be a duplicate of this one. |
comment:19
So should this be closed as duplicate? |
comment:21
If this is not fixed soon, we should just revert #24924. |
comment:23
Note my condition: If this is not fixed soon, we should just revert #24924. The status-quo is not acceptable since this is causing doctest failures. |
comment:24
I know how to fix it (a bit ugly, but mathematically correct). I'll post a branch. |
comment:26
now this works for me both with and without bliss installed. |
comment:27
This does not guarantee correct ordering:
Better use
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:29
ok, fixed |
comment:30
Works for me. |
Changed branch from u/dimpase/blissgeom to |
On 8.3.beta2 with 62 optional packages installed there are failures about automorphism group computations
More precisely
apart from
geometry/lattice_polytope.py
, it's merely different backend producing different, but equivalent, data; the former however uncovers a bug, cf #25426, coming from the work done on #24924.CC: @jplab @mkoeppe @mo271 @kiwifb @stumpc5
Component: group theory
Author: Dima Pasechnik
Branch/Commit:
a23feee
Reviewer: François Bissey
Issue created by migration from https://trac.sagemath.org/ticket/25399
The text was updated successfully, but these errors were encountered: