Opened 5 years ago
Closed 5 years ago
#16433 closed defect (fixed)
clique_with_loops.is_strongly_regular() is True
Reported by:  ncohen  Owned by:  

Priority:  major  Milestone:  sage6.3 
Component:  graph theory  Keywords:  
Cc:  vdelecroix  Merged in:  
Authors:  Nathann Cohen  Reviewers:  Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  4e717d3 (Commits)  Commit:  4e717d3bb11a562c260a34f6031a0def3bec162d 
Dependencies:  Stopgaps: 
Description
In #16370 Vincent reported the following
sage: BIBD = designs.BalancedIncompleteBlockDesign(31,6) sage: V = map(Set, BIBD.blocks()) sage: G = Graph([V, lambda x,y: x&y]) sage: G.is_strongly_regular(parameters=True) (31, 32, 31, 1)
The parameters should always be positive, and this bug comes from the fact that G is a clique with loops. And well, strongly regular graphs are assumed to be simple, so let's keep Sage's code simple without trying to generalize definitions :P
Nathann
Change History (12)
comment:1 Changed 5 years ago by
 Branch set to u/ncohen/16433
 Status changed from new to needs_review
comment:2 Changed 5 years ago by
 Commit set to ea1f5b8888379d2a587b995c04b6bd34bb70ecf5
comment:3 followup: ↓ 4 Changed 5 years ago by
Hi Nathann,
Nice fix! I added docs. See u/vdelecroix/16433. Have a look and put positive review if you like.
Vincent
comment:4 in reply to: ↑ 3 ; followup: ↓ 5 Changed 5 years ago by
Yo !
Nice fix! I added docs. See u/vdelecroix/16433. Have a look and put positive review if you like.
Sounds a bit counterproductive to add such comments here while we implement OrthogonalArrayGraphs
just near. Don't you prefer to have this patch depend on #16370 so that you can link to the doc of OrthogonalArrayGraph
(with stuff like a seealso block) ? By the way we have a LOT of strongly reguar graphs in Sage (including families), so unless you want to make a list of them these comments do not make much sense to me... The doc about BIBD and OA graphs really belong to the doc of those functions, not to the doc of this method.
What we should do perhaps is begin to index the SRG we can build somewhere. But I don't know how to do this properly atm...
So it's up to you. If you insist on adding this I don't mind.
Nathann
comment:5 in reply to: ↑ 4 ; followup: ↓ 6 Changed 5 years ago by
Hi,
Replying to ncohen:
Yo !
Nice fix! I added docs. See u/vdelecroix/16433. Have a look and put positive review if you like.
Sounds a bit counterproductive to add such comments here while we implement
OrthogonalArrayGraphs
just near. Don't you prefer to have this patch depend on #16370 so that you can link to the doc ofOrthogonalArrayGraph
(with stuff like a seealso block) ? By the way we have a LOT of strongly reguar graphs in Sage (including families), so unless you want to make a list of them these comments do not make much sense to me... The doc about BIBD and OA graphs really belong to the doc of those functions, not to the doc of this method.
I do know nothing about strongly regular graphs, so I was very enthusiastic of having this one. But you are right it does not make much sense.
What we should do perhaps is begin to index the SRG we can build somewhere. But I don't know how to do this properly atm...
Ok. Just keep this in mind.
So it's up to you. If you insist on adding this I don't mind.
Just add the doctest about the errors (there are at the very end of the doc in the commit). I do not mind if you do a copy paste and keep it in your branch.
Vincent
comment:6 in reply to: ↑ 5 ; followup: ↓ 7 Changed 5 years ago by
Yo !
I do know nothing about strongly regular graphs, so I was very enthusiastic of having this one. But you are right it does not make much sense.
What about your branch then ?... Do you update it ?
What we should do perhaps is begin to index the SRG we can build somewhere. But I don't know how to do this properly atm...
Ok. Just keep this in mind.
Of course I keep this in mind. I have been implementing families for this only... http://trac.sagemath.org/ticket/14631 http://trac.sagemath.org/ticket/16362
Just add the doctest about the errors (there are at the very end of the doc in the commit). I do not mind if you do a copy paste and keep it in your branch.
Okay Okay so I update your branch ...
Nathann
comment:7 in reply to: ↑ 6 Changed 5 years ago by
Okay, turns out I cannot add a reference to the OA graphs because the ticket may be closed as wontfix. So I do what I can.
Nathann
comment:8 Changed 5 years ago by
 Commit changed from ea1f5b8888379d2a587b995c04b6bd34bb70ecf5 to 4e717d3bb11a562c260a34f6031a0def3bec162d
Branch pushed to git repo; I updated commit sha1. New commits:
4e717d3  trac #16433: more doc

comment:9 Changed 5 years ago by
 Reviewers set to Vincent Delecroix
 Status changed from needs_review to positive_review
Perfect. Thanks.
comment:10 followup: ↓ 11 Changed 5 years ago by
How did you manage to make me the author of the last commit ???
comment:11 in reply to: ↑ 10 Changed 5 years ago by
How did you manage to make me the author of the last commit ???
When you run git commit amend instead of git commit you do not create a new commit but add your changes to the last commit. I just did that. I does not change the author's name.
You can also set the author manually when you do a commit btw.
Nathann
comment:12 Changed 5 years ago by
 Branch changed from u/ncohen/16433 to 4e717d3bb11a562c260a34f6031a0def3bec162d
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
trac #16433: clique_with_loops.is_strongly_regular() is True