Opened 7 years ago

Closed 7 years ago

#13075 closed enhancement (fixed)

Toroidal6RegularGrid2dGraph

Reported by: ncohen Owned by: jason, ncohen, rlm
Priority: major Milestone: sage-5.5
Component: graph theory Keywords:
Cc: Merged in: sage-5.5.rc0
Authors: Nathann Cohen Reviewers: David Coudert
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #13067 Stopgaps:

Description (last modified by ncohen)

Ooch. This patch implements a method with a nasty name. With this one can create 6-regular grids on the torus, and have some acceptable drawing.

It also modifies the embedding of ToroidalGrid2dGraph, as some edges did not appear on the drawing.

Nathann

Attachments (1)

trac_13075.patch (5.6 KB) - added by ncohen 7 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 7 years ago by ncohen

  • Status changed from new to needs_review

comment:2 Changed 7 years ago by eisermbi

  • Status changed from needs_review to needs_info

Hi Nathann,

I looked at the patch and found the modified layout of ToroidalGrid2dGraph better.

For a hexagonal grid graph, can you give some reference with the definition you are using? Your graph is 6-regular while several other definitions are 3-regular. For example, see article 1203.0666v1, section 5, from arXiv.org, or http://mathworld.wolfram.com/HexagonalGrid.html. I am confused ;-)

Birk

comment:3 Changed 7 years ago by ncohen

  • Status changed from needs_info to needs_review

That's rather true... After trying to find an intermediate solution, I ended up renaming the function. It makes more sense like that :-)

I hope that everything is running smoothly on your side ! I am now back from asia and I work in Paris :-)

Nathann

comment:4 Changed 7 years ago by ncohen

  • Description modified (diff)
  • Summary changed from ToroidalHexagonalGrid2dGraph to Toroidal6RegularGrid2dGraph

comment:5 Changed 7 years ago by dcoudert

  • Authors set to Nathann Cohen
  • Reviewers set to David Coudert
  • Status changed from needs_review to needs_work

Hello,

I was about to write a small review patch to add:

  • a space between i and \in (same for j) since otherwise i and in are glued when asking for the doc of the function.
  • to write `((i+1)%n_1, (j+1)%n_2` so add ().

But then I found a problem in the method as well as for ToroidalGrid2dGraph, Grid2dGraph, and certainly others: I'm able to call graphs.Toroidal6RegularGrid2dGraph(-1,2) without warnings! O_o

For which values of n1 and n2 are these methods defined? So far, I'm able to produce a graph that is not 6-regular.

David.

comment:6 Changed 7 years ago by ncohen

  • Status changed from needs_work to needs_review

Oh, right. Well, I added what you asked, in this new function and also in Grid2dGraph :-)

Nathann

comment:7 Changed 7 years ago by dcoudert

  • Status changed from needs_review to positive_review

That's much better now.

comment:8 Changed 7 years ago by ncohen

Thanks ! I'm reimplementing something to compute the girth with static_sparse_graph, because of the conversations on sage-devel these days :-)

It's good to write a piece of code knowing it will not have to be changed later :-)

Nathann

comment:9 Changed 7 years ago by jdemeyer

  • Dependencies changed from 13067 to #13067

comment:10 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-5.4 to sage-5.5

comment:11 Changed 7 years ago by jdemeyer

  • Status changed from positive_review to needs_work

The documentation doesn't build correctly:

/release/merger/sage-5.5.beta2/local/lib/python2.7/site-packages/sage/graphs/graph_generators.py:docstring of sage.graphs.graph_generators.GraphGenerators.Toroidal6RegularGrid2dGraph:16: WARNING: Literal block expected; none found.

and

! Extra }, or forgotten $.
l.85529 ...ordinates $(i,j)$ for $i \in \{0...i-1}
                                                  $
?
! Emergency stop.
l.85529 ...ordinates $(i,j)$ for $i \in \{0...i-1}
                                                  $
!  ==> Fatal error occurred, no output PDF file produced!

comment:12 Changed 7 years ago by ncohen

  • Status changed from needs_work to positive_review

Gloops. LaTeX does not like "%" much, and there was a "::" where it should not be. Sorry about that, I just fixed it !

Nathann

comment:13 Changed 7 years ago by jdemeyer

  • Status changed from positive_review to needs_work

The PDF still doesn't work.

comment:14 Changed 7 years ago by jdemeyer

Are you sure you uploaded the right patch?

comment:15 Changed 7 years ago by ncohen

  • Status changed from needs_work to positive_review
~$ md5sum trac_13075.patch /tmp/a.patch 
2996688ca9b0d1b325e406f054bdca9a  trac_13075.patch
db9c6e30a366712c2c979dfb6805f94f  /tmp/a.patch

Ahem. Sorry again >_<

Nathann

Changed 7 years ago by ncohen

comment:16 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.5.rc0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.