Opened 10 months ago

Closed 8 months ago

#33749 closed enhancement (fixed)

set positions in butterfly digraph generator

Reported by: dcoudert Owned by:
Priority: minor Milestone: sage-9.7
Component: graph theory Keywords:
Cc: Merged in:
Authors: David Coudert Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 3efef4c (Commits, GitHub, GitLab) Commit: 3efef4c29314e22a2c00f32f563aadacaa291309
Dependencies: Stopgaps:

Status badges

Description

We improve the butterfly digraph generator and set the positions of the vertices.

Change History (12)

comment:1 Changed 10 months ago by dcoudert

Branch: public/graphs/33749_butterfly
Commit: 04ca0ae6cbf839c19604b2c62546ebcb9e3fe668
Status: newneeds_review

on the way, we set position of vertices in method Circuit.


New commits:

04ca0aetrac #33749: improve butterfly digraph generator

comment:2 Changed 10 months ago by tscrim

I don't think we should have this doctest:

sage: digraphs.ButterflyGraph(3).show()  # long time

It doesn't fundamentally do anything for testing. I would have a test that outputs the information you want, the position of the vertices for plotting.

comment:3 Changed 9 months ago by git

Commit: 04ca0ae6cbf839c19604b2c62546ebcb9e3fe6688ebbe460122993aac172cce0cf975ef17b643766

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

8ebbe46trac #33749: doctest on position dictionary

comment:4 Changed 9 months ago by dcoudert

Let's try this.

comment:5 Changed 9 months ago by tscrim

Thanks. LGTM.

comment:6 Changed 9 months ago by tscrim

Reviewers: Travis Scrimshaw
Status: needs_reviewpositive_review

comment:7 Changed 9 months ago by vbraun

Status: positive_reviewneeds_work
sage -t --long --warn-long 47.0 --random-seed=123 src/sage/graphs/digraph_generators.py
**********************************************************************
File "src/sage/graphs/digraph_generators.py", line 260, in sage.graphs.digraph_generators.DiGraphGenerators.ButterflyGraph
Failed example:
    digraphs.ButterflyGraph(2).get_pos()
Expected:
    {('11', 0): (0, 0),
     ('11', 1): (2, 0),
     ('11', 2): (4, 0),
     ('10', 0): (0, 1),
     ('10', 1): (2, 1),
     ('10', 2): (4, 1),
     ('01', 0): (0, 2),
     ('01', 1): (2, 2),
     ('01', 2): (4, 2),
     ('00', 0): (0, 3),
     ('00', 1): (2, 3),
     ('00', 2): (4, 3)}
Got:
    {('00', 0): (0, 3),
     ('00', 1): (2, 3),
     ('00', 2): (4, 3),
     ('01', 0): (0, 2),
     ('01', 1): (2, 2),
     ('01', 2): (4, 2),
     ('10', 0): (0, 1),
     ('10', 1): (2, 1),
     ('10', 2): (4, 1),
     ('11', 0): (0, 0),
     ('11', 1): (2, 0),
     ('11', 2): (4, 0)}
**********************************************************************
1 item had failures:
   1 of   6 in sage.graphs.digraph_generators.DiGraphGenerators.ButterflyGraph
    [151 tests, 1 failure, 3.19 s]
----------------------------------------------------------------------
sage -t --long --warn-long 47.0 --random-seed=123 src/sage/graphs/digraph_generators.py  # 1 doctest failed
----------------------------------------------------------------------

comment:8 Changed 9 months ago by git

Commit: 8ebbe460122993aac172cce0cf975ef17b6437663efef4c29314e22a2c00f32f563aadacaa291309

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

777fb19trac #33749: merge with 9.7.beta0
3efef4ctrac #33740: simpler doctest

comment:9 Changed 9 months ago by dcoudert

Status: needs_workneeds_review

Let's avoid sorting issues with a simpler test.

comment:10 Changed 9 months ago by tscrim

Status: needs_reviewpositive_review

Okay. Although I am slightly surprised that there was an issue but a green patchbot. Usually this is sorted correctly and consistently when running the doctests...

comment:11 Changed 9 months ago by dcoudert

I agree this is very surprising. Let's hope we will not hope such issue in other places.

comment:12 Changed 8 months ago by vbraun

Branch: public/graphs/33749_butterfly3efef4c29314e22a2c00f32f563aadacaa291309
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.