Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New non-existence tests for strongly regular graphs #18982

Closed
nathanncohen mannequin opened this issue Aug 3, 2015 · 18 comments
Closed

New non-existence tests for strongly regular graphs #18982

nathanncohen mannequin opened this issue Aug 3, 2015 · 18 comments

Comments

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Aug 3, 2015

This branch adds more infeasibility tests from the following paper:

http://www.win.tue.nl/~aeb/preprints/srgsurvey.pdf

Nathann

Depends on #18960

CC: @dimpase

Component: graph theory

Author: Nathann Cohen

Branch/Commit: 9d85b97

Reviewer: Jori Mäntysalo

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

@nathanncohen nathanncohen mannequin added this to the sage-6.9 milestone Aug 3, 2015
@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 3, 2015

Branch: u/ncohen/18982

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 3, 2015

Commit: 6d72675

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 3, 2015

New commits:

a0173e2trac #18948: Strongly Regular Graphs database
4adcf95trac #18948: Two missing graphs
cf50d70trac #18948: Merged with 6.8
6390dd9trac #18960: Strongly Regular Graphs from two-weight codes
3eee1e7trac #18960: Merge 6.9.beta0
606d15ftrac #18960: Adding nonzero somewhere
83ed332trac #18960: Again...
dc2d326trac #18960: Lint -> vLint
4fb3ce1CURRENT
6d72675trac #18982: New non-existence tests for strongly regular graphs

@nathanncohen nathanncohen mannequin added the s: needs review label Aug 3, 2015
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 3, 2015

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

d9bb797trac #18982: New non-existence tests for strongly regular graphs

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 3, 2015

Changed commit from 6d72675 to d9bb797

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 3, 2015

Changed commit from d9bb797 to 938c769

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 3, 2015

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

938c769trac #18982: New non-existence tests for strongly regular graphs

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Aug 12, 2015

comment:4

Needs rebasing again.

Also when I did raise SomeError('An error occurred.') I got a comment that it should be raise SomeError('an error occurred'). (Which seems stupid to my eyes, but is in some PEP.)

@jm58660 jm58660 mannequin added s: needs work and removed s: needs review labels Aug 12, 2015
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 12, 2015

Changed commit from 938c769 to 9d85b97

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 12, 2015

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

9d85b97trac #18982: Merged with 6.9.beta1

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 12, 2015

comment:6

Needs rebasing again.

Done.

Also when I did raise SomeError('An error occurred.') I got a comment that it should be raise SomeError('an error occurred'). (Which seems stupid to my eyes, but is in some PEP.)

I do not see the point of that either. I hope that your reviewer will not stop here then :-p

Nathann

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Aug 12, 2015

comment:8

Replying to @nathanncohen:

I hope that your reviewer will not stop here then :-p

This is free for Somebody Other(tm) to review... I will add my name to reviewers-field if I make significant progress with reading this.

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Aug 12, 2015

comment:9

Well, was not that hard.

Assuming other parts work, this seems to be direct (and correct) translation of equations from the paper to the code. I read it, and tested with examples given in the paper. I think that l as a variable name makes sense here, because lambda is already reserved name (and we do not use Fortress language :=)), even if it violates PEP 0008 part "Names to Avoid". Hence I mark this as positive review.

Btw, graphs.strongly_regular_graph(324,57,0,12) says "- - graph exists.Comments: - -", i.e. has a missing space. You may want to correct it, when next time modifying this file.

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Aug 12, 2015

Reviewer: Jori Mäntysalo

@dimpase
Copy link
Member

dimpase commented Aug 12, 2015

comment:10

yeah, lambda as a variable name might get you confused...

PS. The tests in the reference are hardly new, I changed wording there.

@dimpase

This comment has been minimized.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 13, 2015

comment:11

Well, was not that hard.

Thanks for the review!

Btw, graphs.strongly_regular_graph(324,57,0,12) says "- - graph exists.Comments: - -", i.e. has a missing space. You may want to correct it, when next time modifying this file.

Done in #19019.

Nathan

@vbraun
Copy link
Member

vbraun commented Aug 13, 2015

Changed branch from u/ncohen/18982 to 9d85b97

@vbraun vbraun closed this as completed in 60b8835 Aug 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants