sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.45k stars 482 forks source link

allow to generate bigger hypergraphs with nauty using genbgL #29821

Closed dimpase closed 4 years ago

dimpase commented 4 years ago

As reported on sage-support, bigger hypergraphs are easy to generate if a different nauty routine is used.

Here we implement this.

CC: @slel @dcoudert

Component: graph theory

Keywords: nauty

Author: Dima Pasechnik

Branch/Commit: 03bd63f

Reviewer: David Coudert, Samuel Lelièvre

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

dimpase commented 4 years ago
comment:1

branch is coming

dimpase commented 4 years ago

Commit: c907930

dimpase commented 4 years ago

Branch: u/dimpase/graphs/genbgL

dimpase commented 4 years ago

New commits:

c907930replace genbg by genbgL, add tests
slel commented 4 years ago
comment:3

Missing closing double-backtick, and maybe spell out "minus":

- at most 64-``number_of_vertices
+ at most 64 minus ``number_of_vertices``

Missing blank line between

    TESTS::

and the test block it introduces.

slel commented 4 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1,5 @@
-as reported on sage-support, bigger hypergraphs are easy to generate if a different nauty rountne is used
+As reported
+[on sage-support](https://groups.google.com/d/topic/sage-support/p5pqzpVf-l8/discussion),
+bigger hypergraphs are easy to generate if a different nauty routine is used.

-here we implement this
+Here we implement this.
slel commented 4 years ago

Changed keywords from none to nauty

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from c907930 to 14d8d51

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

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

14d8d51typos noted by reviewer
dimpase commented 4 years ago
comment:5

ok, fixed

dimpase commented 4 years ago
comment:6

ping?

dcoudert commented 4 years ago
comment:8

may be it's time to update the link to nauty to https://pallini.di.uniroma1.it

also, some pep8 comments

-        if number_of_sets+number_of_vertices > 64:
+        if number_of_sets + number_of_vertices > 64:
-        sp = subprocess.Popen(nautyprefix+"genbgL {0}".format(nauty_input), shell=True,
+        sp = subprocess.Popen(nautyprefix + "genbgL {0}".format(nauty_input), shell=True,
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from 14d8d51 to 03bd63f

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

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

03bd63fPEP8 fixes
dimpase commented 4 years ago
comment:10

fixed PEP8 - and will do the URL changes on nauty update ticket.

dcoudert commented 4 years ago
comment:11

LGTM.

dcoudert commented 4 years ago

Reviewer: David Coudert

dimpase commented 4 years ago
comment:12

I checked that both uniroma and ANU urls are mentioned in SPKG-install.txt And, by the way, please review the nauty update ticket #26891

dimpase commented 4 years ago
comment:13

Replying to @dcoudert:

LGTM.

Thanks

slel commented 4 years ago

Changed reviewer from David Coudert to David Coudert, Samuel Lelièvre

slel commented 4 years ago
comment:14

Thanks!

vbraun commented 4 years ago

Changed branch from u/dimpase/graphs/genbgL to 03bd63f