Opened 2 years ago
Closed 2 years ago
#30303 closed enhancement (fixed)
Graphs: two families of distanceregular graphs
Reported by:  ghIvoMaffei  Owned by:  

Priority:  major  Milestone:  sage9.2 
Component:  graph theory  Keywords:  drg, dual_polar_spaces 
Cc:  dimpase  Merged in:  
Authors:  Ivo Maffei  Reviewers:  Dima Pasechnik 
Report Upstream:  N/A  Work issues:  
Branch:  d7453fa (Commits, GitHub, GitLab)  Commit:  d7453fa0517deeeb16511216f5a91b08b04409d4 
Dependencies:  #30286  Stopgaps: 
Description
We introduce two families of distanceregular graphs related to dual polar spaces.
Change History (13)
comment:1 Changed 2 years ago by
 Branch set to public/graphs/30303
 Commit set to 33bb6a5e15d8aefc19049d0f3fad4885444571bb
 Status changed from new to needs_review
comment:2 Changed 2 years ago by
 Cc dimpase added
comment:3 followup: ↓ 5 Changed 2 years ago by
Why are you using const int m
? any particular reason ?
you must add tests to check the validity of input parameters.
Instead of for v in G.vertices(sort=False)
, you can simply do for v in G
which iterates over the unsorted vertices.,
And instead of G.neighbors(v), you can use
G.neighbor_iterator(v)`.
a few pep8 issues like
(2*m2,q)
>(2 * m  2, q)
({m},{q})
>({m}, {q})
(%d,%d,%d)"%(e,m,q)
>(%d, %d, %d)"%(e, m, q)
comment:4 Changed 2 years ago by
 Commit changed from 33bb6a5e15d8aefc19049d0f3fad4885444571bb to efa934018e31d3af05aec092310a1e51c6ca7d1d
comment:5 in reply to: ↑ 3 ; followup: ↓ 7 Changed 2 years ago by
Replying to dcoudert:
Why are you using
const int m
? any particular reason ?
The idea is that Cython is faster when static typing is used. Not sure how much of a gain is in these pythonfull functions. However, I believe that types are great as they give the reader useful info.
you must add tests to check the validity of input parameters.
Do you mean doctests or checks inside the functions? For the latter there shouldn't be any need as errors will get raised from the functions used inside.
Instead of
for v in G.vertices(sort=False)
, you can simply dofor v in G
which iterates over the unsorted vertices., And instead ofG.neighbors(v), you can use
G.neighbor_iterator(v)`.
These are good things to know and I updated the code. Is there any benefit in using them?
a few pep8 issues
I think I got them all fixed now.
comment:6 Changed 2 years ago by
 Commit changed from efa934018e31d3af05aec092310a1e51c6ca7d1d to be1896327f9ef6bf4c14e5404d3e019f377e49ef
Branch pushed to git repo; I updated commit sha1. New commits:
be18963  added some sig_checks

comment:7 in reply to: ↑ 5 Changed 2 years ago by
Replying to ghIvoMaffei:
Replying to dcoudert:
Why are you using
const int m
? any particular reason ?The idea is that Cython is faster when static typing is used. Not sure how much of a gain is in these pythonfull functions. However, I believe that types are great as they give the reader useful info.
I agree that this way the type of input is checked, but I don't think you can expect any speed up here.
you must add tests to check the validity of input parameters.
Do you mean doctests or checks inside the functions? For the latter there shouldn't be any need as errors will get raised from the functions used inside.
I mean in the function, but if your are sure that functions called inside this method will raise errors in all cases, then it should be ok.
Instead of
for v in G.vertices(sort=False)
, you can simply dofor v in G
which iterates over the unsorted vertices., And instead ofG.neighbors(v), you can use
G.neighbor_iterator(v)`.These are good things to know and I updated the code. Is there any benefit in using them?
for v in G
is equivalent to for v in G.vertex_iterator()
. So it iterates over the vertices without assumption on the order of the vertices.
So far, G.vertices()
returns a (sorted) list of vertices. So when you call for v in G.vertices(sort=False):
, you first build the list of vertices and then iterate over it. It's equivalent to for v in list(G)
or for v in list(G.vertex_iterator())
. Building this list is a waste of space and time here.
This is different for edges as now G.edges()
returns an object of type EdgesView
enabling to iterate over the edges without building the list. So for e in G.edges(sort=False)
and for e in G.edge_iterator()
are now similar. In the future, G.edge_iterator()
will certainly also return a EdgesView
.
I don't have gap installed on my laptop yet, so I let Dima check that every think is ok :P
comment:8 Changed 2 years ago by
Can we have naming scheme and location of DualPolarOrthogonalGraph (to be renamed OrthogonalDualPolarGraph) consistent with UnitaryDualPolarGraph and SymplecticDualPolarGraph,
which are in sage/graphs/generators/classical_geometries.py
?
comment:9 followup: ↓ 11 Changed 2 years ago by
Should I move also the UstimenkoGraph
since is the distance 1or2 graph of the SymplecticDualPolarGraph?
comment:10 Changed 2 years ago by
 Commit changed from be1896327f9ef6bf4c14e5404d3e019f377e49ef to d7453fa0517deeeb16511216f5a91b08b04409d4
comment:11 in reply to: ↑ 9 Changed 2 years ago by
Replying to ghIvoMaffei:
Should I move also the
UstimenkoGraph
since is the distance 1or2 graph of the SymplecticDualPolarGraph?
No, I think it's OK with UstimenkoGraph as it is now.
comment:12 Changed 2 years ago by
 Reviewers set to Dima Pasechnik
 Status changed from needs_review to positive_review
looks good, thanks.
comment:13 Changed 2 years ago by
 Branch changed from public/graphs/30303 to d7453fa0517deeeb16511216f5a91b08b04409d4
 Resolution set to fixed
 Status changed from positive_review to closed
Last 10 new commits:
fix wrong rebase merge conflict resolution
Merge branch 'drg_sporadic2' into drg_sporadic3
added the last sporadic drg
added references; added name to graph catalog
fix some docstrings
added gap_packages flag to atlasrep
Merge branch 'drg_sporadic1' into drg_sporadic2
added gap_packages flag to atlasrep
Merge ticket 30260 into ticket 30286
added dual polar orthogonal and Ustimenko