#19224: swtich OA(k,n)+* strongly regular graphs
-------------------------+-------------------------------------------------
Reporter: | Owner:
ncohen | Status: needs_work
Type: | Milestone: sage-6.9
enhancement | Resolution:
Priority: major | Merged in:
Component: graph | Reviewers:
theory | Work issues:
Keywords: | Commit:
Authors: | 9fc78a492823ede303f45a4a6238d06a0c20f87e
Nathann Cohen | Stopgaps:
Report Upstream: N/A |
Branch: |
u/ncohen/19224 |
Dependencies: |
-------------------------+-------------------------------------------------
Comment (by ncohen):
Hello,
> 1. You wrote
> {{{
> from math import sqrt
> cdef int n = int(sqrt(n_2_p_1-1))
> }}}
I did that because sqrt(-4) raises an exception, while in C floor(-a)
returns nan. I prefer this behaviour when I do not need the speed. As a
reviewer, your job is to make sure that the code is correct an reliable,
properly documented etc. These kind of comments are to me a matter of
taste, and it would be cool if you proposed a commit rather than include
as part of your review that the way it is done does not conform to your
taste. As there isn't, I believe, any reason to prefer yours over mine in
this context where the speed difference does not matter.
> 2. Why are you testing withtout the option `resolvable=True`
That's a mistake indeed.
> 3. Use better spacings: `int v,int k,int l` -> `int v, int k, int l`.
As previously, please propose a commit the next time you cannot tolerate a
missing space.
> 4. Do you mind moving out the function `switch_OA_srg` into a global
function? With such local definition you are generating again and again
different Python functions with the very same code
It is not that I would mind, but mostly that I would not know how to write
its documentation to my taste. The subfunction takes `c` and `n` as
parameters, whose role has to be explained. Plus 'c' is actually used
twice in the function in "different ways" (it has two different meanings
in the construction), so that the independent function should probably
differenciate the two uses of 'c' instead of tying them to be equal (and I
have no use for that generalization). The result would be unclear, look
hardcoded, and the name of the function is hard to pick too as this class
does not seem very standard.
More technically, I could not care less about returning functions which
are not equal to each other. My job here is to build new graphs.
> 5. Could you add an example of a SRG that was not available before?
What about the 4 that already appear in the doctest?
Nathann
--
Ticket URL: <http://trac.sagemath.org/ticket/19224#comment:4>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
