#19418: skew-Hadamard matrices and related srg's
-------------------------+-------------------------------------------------
Reporter: | Owner:
dimpase | Status: needs_work
Type: | Milestone: sage-6.10
enhancement | Resolution:
Priority: major | Merged in:
Component: | Reviewers: Nathann Cohen
combinatorics | Work issues:
Keywords: | Commit:
Authors: Dima | 01763fd38506f8721560fa29e60227e5081db08b
Pasechnik | Stopgaps:
Report Upstream: N/A |
Branch: |
public/19418 |
Dependencies: |
#19309 |
-------------------------+-------------------------------------------------
Changes (by ncohen):
* status: needs_review => needs_work
Comment:
Helloooooooooo Dima,
Here is another pass:
- I don't think that you need to import functions at the top of every
single
'::' block. If you imported them in a previous doctest of the same
function
they are usually around. I say 'usually' because I don't think I read
that
anywhere, but it just works. If the order in which doctests are run can
be
'random', I think that it just shuffles the functions and not the
doctests
inside of a function.
- docstring of zero_position: bad alignment of 'place it first'. Misses
one
space.
- Same line: the sentence is not complete -- what about adding 'When it'
at its
beginning?
- You did not doctest your new flag. Also, the code *looks* wrong.
- Docstring of `williamson_goethals_seidel_skew_hadamard_matrix`. Things
seem to
miss from the first sentence of the paragraph. Or it's me being sick
again.
P.S.: syntactically, a ')' is missing at least.
- Same function. The following doc should be in the function that actually
returns those matrices:
{{{
+ Matrices for `n=36` and `52` are given in [GS70s]_. Matrices for
`n=92` are given
+ in [Wall71]_.
}}}
- `_GS_skew_hadamard` -- the name of that function is a bit too vague.
What
about `GS_skew_hadamard_smallcases`? You don't necessarily need to make
it
private, by the way. It lives in a module and does not appear in the
global
namespace, so only the interested guys will find it. And it's not
technically
complicated to use, the input/output is well defined and reliable. It
can't be
misused.
- private functions need doctests too. 'cdef' is the only situation in
which you
don't need them.
- `def pmtoZ(s):` could be defined in the only 'if' that requires it (not
very
important).
- docstring of `skew_hadamard_matrix` -- in the block of doc about
'existence',
one reads 'but that design does not exist'. Could you replace 'design'
by
'matrix'?
- I don't see why the following doctest has a 'long' flag
{{{
sage: skew_hadamard_matrix(784,existence=True) # long time
}}}
If you actually built the matrix, however, that would be correct.
- I don't think that `check=False` is useful when `existence=True`
{{{
skew_hadamard_matrix(n//2,existence=True, check=False)
}}}
- In the `n%8==0` case you implement the product construction of skew
hadamard
matrices with a 2x2 matrix only. Is there a reason to that?
- `if _GS_skew_hadamard(n, existence=True)` -- it hardly matters, but it
would
be 'cleaner' to call this (quick) function before checking the more
complicated (recursive) constructions.
- Shouldn't you also check that the matrix is 'skew-normalized', as we do
with
the regular normalization?
{{{
+ if skew_normalize:
+ dd = diagonal_matrix(M[0])
+ M = dd*M*dd
+ if check:
+ assert is_hadamard_matrix(M, normalized=False, skew=True)
}}}
- If you are too humble to do it yourself, I will change that to 'Return
the
Pasechnik graph of order n' `:-)`
{{{
+ Pseudo-`OA(2n-1,4n-1)`-graph from a skew Hadamard matrix of order
`4n`
}}}
- Please add your new graphs to the `graphs.<tab>` constructor.
Nathann
--
Ticket URL: <http://trac.sagemath.org/ticket/19418#comment:37>
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.
