sagemath / sage

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

Symplectic graphs #14532

Closed 6bdad4c1-1e26-4f2f-a442-a01a2292c181 closed 11 years ago

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 11 years ago

Brand new pretty graphs :-P

Nathann

Apply :

Depends on #14217

CC: @sagetrac-azi

Component: graph theory

Keywords: strongly regular graphs

Author: Nathann Cohen

Reviewer: Frédéric Chapoton

Merged: sage-5.10.beta4

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

ca0b67cc-9d10-44c6-be51-5d9e2cdee96a commented 11 years ago
comment:2

Hello!

Sry for comming back at you so late. I had some other stuff lately!

Anways, the code looks fine (read it, run it, and obviously run sage -t) I only have two remarks.

  1. Maybe we should check if d is even and also nonzero (perhaps >= 2) since that is also a invalid parameter.

  2. I would change the line

 Graph([map(tuple,PV), lambda x,y:V(x)*(M*V(y)) == 0], loops = False) 
}}} to

{{{
{{{
 Graph((map(tuple,PV), lambda x,y:V(x)*(M*V(y)) == 0), loops = False) 
}}} 
}}}

which should be 2x faster.

Best,

Jernej
6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 11 years ago
comment:3

Helloooooooooooooo !!

Sry for comming back at you so late. I had some other stuff lately!

Come on, I am already very thankful that you take the time to review these patches !!!

  1. Maybe we should check if d is even and also nonzero (perhaps >= 2) since that is also a invalid parameter.

Done

  1. I would change the line
 Graph([map(tuple,PV), lambda x,y:V(x)*(M*V(y)) == 0], loops = False) 
}}} to

{{{
{{{
 Graph((map(tuple,PV), lambda x,y:V(x)*(M*V(y)) == 0), loops = False) 
}}} 
}}}

which should be 2x faster.

O_o

Here is what I get when I change it :

sage: graphs.SymplecticGraph(4,4)         
...
NetworkXError: Input is not a known data type for conversion.

But what do you think it should do, and why do you think that it should be 2x faster ? O_o

Nathann

ca0b67cc-9d10-44c6-be51-5d9e2cdee96a commented 11 years ago
comment:4

I may be shooting random nonsense of course but in general it is much better to not create lists [] but iterators (). Since in the former case a list has to first be created (1 for loop) and then iterated over (2 for loop). Example

sage: %timeit max((i for i in xrange(100)))
100000 loops, best of 3: 14.5 us per loop
sage: %timeit max([i for i in xrange(100)])
10000 loops, best of 3: 30 us per loop

and even faster in this case would be

sage: %timeit 99
1000000 loops, best of 3: 429 ns per loo
6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 11 years ago
comment:5

Yepyep but no list is created in this case. A list of size 2 is given to the Graph constructor : the first element is a list of vertices, the second element is a function that gives adjacent pairs.

Nathann

ca0b67cc-9d10-44c6-be51-5d9e2cdee96a commented 11 years ago
comment:6

Oh FML you see sometimes I do shoot random nonsense!

In this case the patch is ofc OK. If I were to do this I would make the additional test check (since we're already doing them) but its fine as is as well.

fchapoton commented 11 years ago
comment:7

there is a typo in "simplectic" at least twice

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 11 years ago
comment:8

there is a typo in "simplectic" at least twice

Arggggggggg... Fixed :-P

Most probably because of that cursed sImplex :-P

Nathann

fchapoton commented 11 years ago
comment:9

Attachment: trac_14532.patch.gz

hello,

if you are happy with my review patch (just removing unused imports), you can set a positive review on my behalf.

fchapoton commented 11 years ago

Attachment: trac_14532_review-fc.patch.gz

fchapoton commented 11 years ago

Changed keywords from none to strongly regular graphs

fchapoton commented 11 years ago

Reviewer: Frédéric Chapoton

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 11 years ago
comment:11

All tests pass ! Thank you very much for your help :-)

Nathann

jdemeyer commented 11 years ago
comment:12
sage -t devel/sage/sage/graphs/generators/families.py
**********************************************************************
File "devel/sage/sage/graphs/generators/families.py", line 1992, in sage.graphs.generators.families.SymplecticGraph
Failed example:
    g = graphs.SymplecticGraph(6,2)
Exception raised:
    Traceback (most recent call last):
      File "/mazur/release/merger/sage-5.10.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 466, in _run
        self.execute(example, compiled, test.globs)
      File "/mazur/release/merger/sage-5.10.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 825, in execute
        exec compiled in globs
      File "<doctest sage.graphs.generators.families.SymplecticGraph[0]>", line 1, in <module>
        g = graphs.SymplecticGraph(Integer(6),Integer(2))
      File "/mazur/release/merger/sage-5.10.beta3/local/lib/python2.7/site-packages/sage/graphs/generators/families.py", line 2000, in SymplecticGraph
        from sage.schemes.generic.projective_space import ProjectiveSpace
    ImportError: No module named projective_space
**********************************************************************
jdemeyer commented 11 years ago

Dependencies: #14217

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 11 years ago
comment:14

Attachment: trac_14532-rebased.patch.gz

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 11 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1,9 @@
 Brand new pretty graphs `:-P`

 Nathann
+
+Apply :
+
+* [attachment: trac_14532.patch](https://github.com/sagemath/sage-prod/files/10657725/trac_14532.patch.gz)
+* [attachment: trac_14532_review-fc.patch](https://github.com/sagemath/sage-prod/files/10657726/trac_14532_review-fc.patch.gz)
+* [attachment: trac_14532-rebased.patch](https://github.com/sagemath/sage-prod/files/10657727/trac_14532-rebased.patch.gz)
6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 11 years ago
comment:15

Rebased !

Nathann

jdemeyer commented 11 years ago

Merged: sage-5.10.beta4

dimpase commented 11 years ago
comment:17

maybe Sage should have "polar space" graphs in general, not, only symplectic ones?

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 11 years ago
comment:18

Helloooooooo !

maybe Sage should have "polar space" graphs in general, not, only symplectic ones?

Well, yes it would be nice indeed, but I do not know how to build them. I guess that it only takes 5~6 lines, like for the symplectic ones, but I don't know which ones. Actually, I have no idea on earth what these graphs are, except what I could read (and understand, which is even less) from Brouwer's website.

http://www.win.tue.nl/~aeb/graphs/srg/srgtab.html

I believe I created what he calls a VO^- graph (graphs.BrouwerHaemersGraph), and the same code worked for different parameters but I was not able to make it work in characteristic two, and so I did not write this more general patch.

If you know how to make it work, I would be glad to see it in Sage too :-P

Nathann

dimpase commented 11 years ago
comment:19

Replying to @nathanncohen:

Helloooooooo !

maybe Sage should have "polar space" graphs in general, not, only symplectic ones?

Well, yes it would be nice indeed, but I do not know how to build them. I guess that it only takes 5~6 lines, like for the symplectic ones, but I don't know which ones. Actually, I have no idea on earth what these graphs are, except what I could read (and understand, which is even less) from Brouwer's website.

http://www.win.tue.nl/~aeb/graphs/srg/srgtab.html

I believe I created what he calls a VO^- graph (graphs.BrouwerHaemersGraph), and the same code worked for different parameters but I was not able to make it work in characteristic two, and so I did not write this more general patch.

no, these are different species. I mean classical polar spaces as introduced by J.Tits (or even long before him). See e.g. Sect 6.5 of http://www.maths.qmul.ac.uk/~pjc/pps/pps6.pdf

To construct these, you need to be able to create the corresponding forms, which are well-studied by group theory, as they lead to finite classical groups. GAP has code to create these forms; it's not completely trivial in characteristic two. You can actually just call GAP! E.g.

gap> Display(InvariantQuadraticForm(GO(1,6,2)).matrix);
 . 1 . . . .
 . . . . . .
 . . . 1 . .
 . . . . . .
 . . . . . 1
 . . . . . .
gap> Display(InvariantQuadraticForm(GO(-1,6,2)).matrix);
 . 1 . . . .
 . . . . . .
 . . 1 1 . .
 . . . 1 . .
 . . . . . 1
 . . . . . .
gap> Display(InvariantSesquilinearForm(GU(6,2)).matrix);
 . . . . . 1
 . . . . 1 .
 . . . 1 . .
 . . 1 . . .
 . 1 . . . .
 1 . . . . .
gap> Display(InvariantBilinearForm(Sp(6,3)).matrix);
 . . . . . 1
 . . . . 1 .
 . . . 1 . .
 . . 2 . . .
 . 2 . . . .
 2 . . . . .
gap> 

etc...

dimpase commented 11 years ago
comment:20

The more one goes down this road, the more the lack of a proper backend for graphs with big automorphism groups shows. Perhaps we can try implement something next month in Paris.

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 11 years ago
comment:21

The more one goes down this road, the more the lack of a proper backend for graphs with big automorphism groups shows. Perhaps we can try implement something next month in Paris.

Ahahah. Well, why not ? But I really know next to nothing about those, and what people use them for :-)

Nathann

dimpase commented 11 years ago
comment:22

Replying to @nathanncohen:

The more one goes down this road, the more the lack of a proper backend for graphs with big automorphism groups shows. Perhaps we can try implement something next month in Paris.

Ahahah. Well, why not ? But I really know next to nothing about those, and what people use them for :-)

it's useful to

Isn't it obvious? All these graphs you construct lately have huge automorphism groups, often arc-transitive and/or distance-transitive.

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 11 years ago
comment:23

Isn't it obvious?

What do you use them for ? What do you want to compute ? Graphs have a lot of method, you know.. :-P

Nathann

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 11 years ago
comment:24

By the way, and because these graphs are usually immutable (and dense), I wrote a couple of C functions (#14589) to store very compactly an adjacency matrix. Of course it cannot compare with an encoding by generators of the automorphism group, but a graph on 30 000 vertices can be stored on 128MB.

Nathann

dimpase commented 11 years ago
comment:25

Replying to @nathanncohen:

Isn't it obvious?

What do you use them for ? What do you want to compute ? Graphs have a lot of method, you know.. :-P

Obviously, regularity properties - and this is very fast with such data. Then, e.g., e.g. maximum cliques, or an optimal colouring. It's downright hopeless to do without taking symmetries into account. Or Lovasz theta number...

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 11 years ago
comment:26

Obviously, regularity properties - and this is very fast with such data. Then, e.g., e.g. maximum cliques, or an optimal colouring. It's downright hopeless to do without taking symmetries into account. Or Lovasz theta number...

Hmmm... Looks like you will have an awful amount of code to write :-P

Nathann

dimpase commented 11 years ago
comment:27

Replying to @nathanncohen:

Obviously, regularity properties - and this is very fast with such data. Then, e.g., e.g. maximum cliques, or an optimal colouring. It's downright hopeless to do without taking symmetries into account. Or Lovasz theta number...

Hmmm... Looks like you will have an awful amount of code to write :-P

I wouldn't classify calls to GAP as "awful amount of code" :)

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 11 years ago
comment:28

Oh ! Well, if it's all in there already, then.... :-)

Nathann

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 11 years ago
comment:29

To construct these, you need to be able to create the corresponding forms, which are well-studied by group theory, as they lead to finite classical groups. GAP has code to create these forms; it's not completely trivial in characteristic two. You can actually just call GAP!

This is now #14631 !

Nathann