sagemath / sage

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

Add interface to nauty-genbg (generator of bipartite graphs with given bipartition) #33365

Closed maxale closed 2 years ago

maxale commented 2 years ago

We add missing interface to nauty-genbg, a generator of bipartite graphs.

Nauty's genbg was already compiled and accessible but the interface was missing.

Depends on #33366

CC: @tscrim

Component: graph theory

Author: David Coudert

Branch: 695a667

Reviewer: Travis Scrimshaw

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

dcoudert commented 2 years ago

Commit: 804b062

dcoudert commented 2 years ago

Dependencies: #33366

dcoudert commented 2 years ago

New commits:

3603be3Create bipartite graph from graph6 string
2175d09Change Alist file check, Add docs
12a598cChange alist file check
6d70523Remove docstring newline
8090e50trac #33365: merged #33366 with 9.6.beta7
804b062trac #33365: add interface to nauty genbg
dcoudert commented 2 years ago

Description changed:

--- 
+++ 
@@ -1 +1,4 @@
-Sage provides interface to `nauty-geng` tool via `graph.nauty_geng()`, but interface for `nauty-genbg` tool is missing. 
+We add missing interface to `nauty-genbg`, a generator of bipartite graphs.
+
+Nauty's genbg was already compiled and accessible but the interface was missing. 
+
dcoudert commented 2 years ago

Branch: public/graphs/33365_genbg

dcoudert commented 2 years ago

Author: David Coudert

maxale commented 2 years ago
comment:3

I see that the resulting graphs are created via

         G = BipartiteGraph(s[:-1], format='graph6')

Please notice that graph6 format does not bear bipartition information, and the bipartition produced by BipartiteGraph() may be different from the requested one (via parameters n1 and n2), especially for non-connected ones, where multiple bipartitions may exist. I believe genbg produces the graph under assumption that left and right partite sets are formed by vertices (0..n1-1) and (n1..n1+n2-1), respectively. This needs to be enforced for the resulting graphs like:

         G = BipartiteGraph(s[:-1], format='graph6', partition=(set(0..n1-1),set(n1..n1+n2-1)))

I did not test this, but I hope the idea is clear.

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

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

e57e456trac #33365: ensure that the partition respects the requirement
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 804b062 to e57e456

dcoudert commented 2 years ago
comment:5

You are right. Not easy when n1 and n2 are somewhere in the options string...

I'm not particularly proud of my solution of parsing the auxiliary output, but it's working for as long as the user don't pass option -q (which suppress the auxiliary output).

dcoudert commented 2 years ago
comment:6

green bot, please review.

tscrim commented 2 years ago

Reviewer: Travis Scrimshaw

tscrim commented 2 years ago
comment:8

More of a nitpick: I think (-s, -a) and, e.g., ">M" should be code-formatted.

I agree that it is not that elegant, but it seems to work. If issues arise down the road, we can fix them then.

Whether or not you include my above nitpick, you can set a positive review on my behalf.

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

Changed commit from e57e456 to 695a667

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

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

1e80907trac #33365: Merged with 9.7.beta2
695a667trac #33365: formatting
dcoudert commented 2 years ago
comment:10

I'm not sure to understand what you asked for... I changed to (``-s``, ``-a``).

I prefer to avoid mentioning ">M". It can effectively be used to count graphs, but it's not documented and does not even appear in the source code of genbg.

dcoudert commented 2 years ago
comment:11

As proposed, I set this ticket to positive review.

Thanks.

tscrim commented 2 years ago
comment:12

Replying to @dcoudert:

I'm not sure to understand what you asked for... I changed to (``-s``, ``-a``).

Yes, that’s right: code formatting is ``.

I prefer to avoid mentioning ">M". It can effectively be used to count graphs, but it's not documented and does not even appear in the source code of genbg.

Those kind of things were included in the doc though. Anyways, not so important.

Thank you.

vbraun commented 2 years ago

Changed branch from public/graphs/33365_genbg to 695a667

maxale commented 2 years ago

Changed commit from 695a667 to none

maxale commented 2 years ago
comment:14

Please notice that nauty-genbg has the following restrictions on n1 and n2:

$ nauty-genbg 5 28
>E genbg: must have n1=1..24, n1+n2=1..32

Should these restrictions be checked by the interface and an appropriate error be raised?

dcoudert commented 2 years ago
comment:15

Yes, and these restrictions should be properly documented. Can you open a ticket ?

maxale commented 2 years ago
comment:16

Added as ticket #34179.