Sage: Ticket #19340: Better interface for hadamard_matrix
https://trac.sagemath.org/ticket/19340
<p>
This branch improves the interface of hadamard_matrix.
</p>
<ul><li>It adds it in <code>matrix.<tab></code>
</li><li>It checks the returned results ('check' flag)
</li><li>It can test for the existence of a construction in Sage (useful for SRG code)
</li></ul><p>
Nathann
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/19340
Trac 1.1.6ncohenSat, 03 Oct 2015 16:09:46 GMTstatus changed; commit, branch set
https://trac.sagemath.org/ticket/19340#comment:1
https://trac.sagemath.org/ticket/19340#comment:1
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>commit</strong>
set to <em>f465c96099ab3681ba1162eaf739c7768d8218ab</em>
</li>
<li><strong>branch</strong>
set to <em>u/ncohen/19340</em>
</li>
</ul>
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=f465c96099ab3681ba1162eaf739c7768d8218ab"><span class="icon"></span>f465c96</a></td><td><code>trac #19340: Better interface for hadamard_matrix</code>
</td></tr></table>
TicketdimpaseSat, 03 Oct 2015 16:20:02 GMT
https://trac.sagemath.org/ticket/19340#comment:2
https://trac.sagemath.org/ticket/19340#comment:2
<p>
IMHO <code>hadamard_matrix()</code> should accept options, such as <code>RSHCD</code> and <code>skew</code> (the latter not implemented yet). (A skew-Hadamard matrix is a Hadamard matrix of the form <code>H=S+I</code>, with <code>S.T==-S</code> and <code>I</code> the identity matrix; I have some half-written patch for them...)
</p>
<p>
As well, <code>is_hadamard_matrix()</code> ought to check for <code>RSHCD</code>-ness and <code>skew</code>-ness.
</p>
TicketncohenSat, 03 Oct 2015 16:24:01 GMT
https://trac.sagemath.org/ticket/19340#comment:3
https://trac.sagemath.org/ticket/19340#comment:3
<blockquote class="citation">
<p>
IMHO <code>hadamard_matrix()</code> should accept options, such as <code>RSHCD</code> and <code>skew</code> (the latter not implemented yet).
</p>
</blockquote>
<p>
HMmmmm.. I don't know for 'skew', but 'rshcd' sounds very very specific to have in such a general function. Having a function of its own in a submodule sounds sufficient to me, for the moment.
</p>
<blockquote class="citation">
<p>
As well, <code>is_hadamard_matrix()</code> ought to check for <code>RSHCD</code>-ness and <code>skew</code>-ness.
</p>
</blockquote>
<p>
I don't know... The rshcd function already tests the rshcd-specific property anyway.
</p>
<p>
Nathann
</p>
TicketgitSat, 03 Oct 2015 16:28:54 GMTcommit changed
https://trac.sagemath.org/ticket/19340#comment:4
https://trac.sagemath.org/ticket/19340#comment:4
<ul>
<li><strong>commit</strong>
changed from <em>f465c96099ab3681ba1162eaf739c7768d8218ab</em> to <em>e02902ae0c073a13700295ce75ec1db2548a7b6d</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=e02902ae0c073a13700295ce75ec1db2548a7b6d"><span class="icon"></span>e02902a</a></td><td><code>trac #19340: Pre-existing bug that hid several paley constructions</code>
</td></tr></table>
TicketgitSat, 03 Oct 2015 16:48:23 GMTcommit changed
https://trac.sagemath.org/ticket/19340#comment:5
https://trac.sagemath.org/ticket/19340#comment:5
<ul>
<li><strong>commit</strong>
changed from <em>e02902ae0c073a13700295ce75ec1db2548a7b6d</em> to <em>247a2bb4dcec180227cd9fb9bcd331a27a289016</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=247a2bb4dcec180227cd9fb9bcd331a27a289016"><span class="icon"></span>247a2bb</a></td><td><code>trac #19340: Pre-existing bug that hid several paley constructions</code>
</td></tr></table>
Ticketfredrik.johanssonSat, 03 Oct 2015 18:34:56 GMT
https://trac.sagemath.org/ticket/19340#comment:6
https://trac.sagemath.org/ticket/19340#comment:6
<p>
You might consider using the flint functions fmpz_mat_hadamard and fmpz_mat_is_hadamard, which should be faster.
</p>
<p>
Checking by default seems kind of useless since this is much slower than generating the matrix (generating a size 1000 Hadamard matrix takes 0.01 seconds and checking it takes 0.5 seconds), and unit testing should provide reasonable certainty that the algorithm is correct. Flint's test suite already checks for every size up to 300 that fmpz_mat_hadamard either constructs a correct Hadamard matrix or reports that the size is unsupported.
</p>
TicketncohenSat, 03 Oct 2015 18:43:28 GMT
https://trac.sagemath.org/ticket/19340#comment:7
https://trac.sagemath.org/ticket/19340#comment:7
<blockquote class="citation">
<p>
You might consider using the flint functions fmpz_mat_hadamard and fmpz_mat_is_hadamard, which should be faster.
</p>
</blockquote>
<p>
How easily can I call them on a Sage matrix?
</p>
<blockquote class="citation">
<p>
Checking by default seems kind of useless since this is much slower than generating the matrix (generating a size 1000 Hadamard matrix takes 0.01 seconds and checking it takes 0.5 seconds), and unit testing should provide reasonable certainty that the algorithm is correct.
</p>
</blockquote>
<p>
Sorry to disappoint, but I have thousands of line behind me (in combinat/designs/) that implement old combinatorial designs from lost papers. I know the value of this kind of testing.
</p>
<blockquote class="citation">
<p>
Flint's test suite already checks for every size up to 300 that fmpz_mat_hadamard either constructs a correct Hadamard matrix or reports that the size is unsupported.
</p>
</blockquote>
<p>
Are you saying that flint can be easily accessed from Sage *and* that it contains a database of hadamard matrices? If so, I woulld be glad to call it from the general constructor of hadamard matrices in Sage.
</p>
<p>
Nathann
</p>
Ticketfredrik.johanssonSat, 03 Oct 2015 19:20:14 GMT
https://trac.sagemath.org/ticket/19340#comment:8
https://trac.sagemath.org/ticket/19340#comment:8
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/19340#comment:7" title="Comment 7">ncohen</a>:
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
You might consider using the flint functions fmpz_mat_hadamard and fmpz_mat_is_hadamard, which should be faster.
</p>
</blockquote>
<p>
How easily can I call them on a Sage matrix?
</p>
</blockquote>
<p>
It should be enough (untested) to add the following declarations in libs/flint/fmpz_mat.pxd
</p>
<pre class="wiki"> int fmpz_mat_hadamard(fmpz_mat_t A)
int fmpz_mat_is_hadamard(const fmpz_mat_t A)
</pre><p>
and the following methods in matrix/matrix_integer_dense.pyx
</p>
<pre class="wiki"> def _hadamard(self):
# in-place, returns whether successful
return fmpz_mat_hadamard(self._matrix)
def is_hadamard(self):
return fmpz_mat_is_hadamard(self._matrix)
</pre><p>
Then you can do something like this in hadamard_matrix()
</p>
<pre class="wiki"> if not(n % 4 == 0) and (n > 2):
raise ValueError
mat = Matrix(ZZ, n, n)
if mat._hadamard():
return mat
else:
# use database here
raise ValueError
</pre><blockquote class="citation">
<blockquote class="citation">
<p>
Checking by default seems kind of useless since this is much slower than generating the matrix (generating a size 1000 Hadamard matrix takes 0.01 seconds and checking it takes 0.5 seconds), and unit testing should provide reasonable certainty that the algorithm is correct.
</p>
</blockquote>
<p>
Sorry to disappoint, but I have thousands of line behind me (in combinat/designs/) that implement old combinatorial designs from lost papers. I know the value of this kind of testing.
</p>
</blockquote>
<p>
You may well be right about combinatorial designs in general. But, having written the flint code, and considering that it implements a fairly simple algorithm, when it's already exhaustively tested up to size 300, I think it's highly unlikely to give an incorrect matrix for larger sizes. So I can't agree with slowing it down many orders of magnitude by default.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
Flint's test suite already checks for every size up to 300 that fmpz_mat_hadamard either constructs a correct Hadamard matrix or reports that the size is unsupported.
</p>
</blockquote>
<p>
Are you saying that flint can be easily accessed from Sage *and* that it contains a database of hadamard matrices? If so, I woulld be glad to call it from the general constructor of hadamard matrices in Sage.
</p>
</blockquote>
<p>
It doesn't have a database; it uses the Paley construction. As noted above, you can easily use a fallback database/algorithm if it fails.
</p>
<p>
BTW, fmpz_mat_hadamard succeeds for every multiple of 4 below 92; current code for the Paley construction in Sage doesn't succeed to construct a size 20 Hadamard matrix.
</p>
TicketncohenSat, 03 Oct 2015 19:45:28 GMT
https://trac.sagemath.org/ticket/19340#comment:9
https://trac.sagemath.org/ticket/19340#comment:9
<blockquote class="citation">
<p>
It should be enough (untested) to add the following declarations in libs/flint/fmpz_mat.pxd
</p>
</blockquote>
<p>
HMmmm.. I don't feel much at ease with adding matrix code, sorry. Especially if it is only implemented for a specific data structure and such.
</p>
<blockquote class="citation">
<p>
You may well be right about combinatorial designs in general. But, having written the flint code, and considering that it implements a fairly simple algorithm, when it's already exhaustively tested up to size 300, I think it's highly unlikely to give an incorrect matrix for larger sizes. So I can't agree with slowing it down many orders of magnitude by default.
</p>
</blockquote>
<p>
Unless you do intend to use this code, couldn't you trust the opinion of those who do and develop it?
</p>
<p>
I know how many hours, night and weeks I wasted because of a typo in data or in code. You wouldn't mind slowing by "several orders of magnitude" a function like <code>.__repr__</code> because it is never a critical. Do you generate Hadamard matrices intensively? Furthermore you can disable the check with 'check=False'. Please don't turn this ticket into a war just because of a check you can disable, which saved my skin innumerable times in similar code. It's not worth it. I know it, and I need the flag. Besides, checking the constructions is already the default for all other design constructions.
</p>
<blockquote class="citation">
<p>
It doesn't have a database; it uses the Paley construction. As noted above, you can easily use a fallback database/algorithm if it fails.
</p>
</blockquote>
<p>
More constructions of hadamard matrices may come. Others may be pure data. Other may be recursive constructions on both.
</p>
<blockquote class="citation">
<p>
BTW, fmpz_mat_hadamard succeeds for every multiple of 4 below 92; current code for the Paley construction in Sage doesn't succeed to construct a size 20 Hadamard matrix.
</p>
</blockquote>
<p>
Then there must be another bug somewhere. This code clearly needs some testing.
</p>
<p>
Nathann
</p>
TicketncohenSat, 03 Oct 2015 20:02:56 GMT
https://trac.sagemath.org/ticket/19340#comment:10
https://trac.sagemath.org/ticket/19340#comment:10
<blockquote class="citation">
<blockquote class="citation">
<p>
BTW, fmpz_mat_hadamard succeeds for every multiple of 4 below 92; current code for the Paley construction in Sage doesn't succeed to construct a size 20 Hadamard matrix.
</p>
</blockquote>
</blockquote>
<p>
Two updates:
</p>
<p>
1) With this branch, Sage can build a matrix for n=20. It's one of the bugs I fixed
2) There is no Paley construction for 52 on [1]. Either you get the data somewhere else or there is something wrong in that code.
</p>
<p>
Nathann
</p>
<p>
[1] <a class="ext-link" href="http://neilsloane.com/hadamard/"><span class="icon"></span>http://neilsloane.com/hadamard/</a>
</p>
Ticketfredrik.johanssonSat, 03 Oct 2015 20:14:52 GMT
https://trac.sagemath.org/ticket/19340#comment:11
https://trac.sagemath.org/ticket/19340#comment:11
<p>
<code>52 = 2 * (5^2 + 1)</code> is covered by the "type 2" Paley construction.
</p>
<p>
The list you linked to is evidently incomplete.
</p>
TicketncohenSat, 03 Oct 2015 20:16:36 GMT
https://trac.sagemath.org/ticket/19340#comment:12
https://trac.sagemath.org/ticket/19340#comment:12
<p>
Then that's another bug of the current code:
</p>
<pre class="wiki">sage: hadamard_matrix_paleyII(52)
...
ValueError: The order 52 is not covered by the Paley type II construction.
</pre>
Ticketfredrik.johanssonSat, 03 Oct 2015 20:18:38 GMT
https://trac.sagemath.org/ticket/19340#comment:13
https://trac.sagemath.org/ticket/19340#comment:13
<p>
Well, it's up to you whether you want to fix the broken code or just insert a few lines to call the code in flint that is faster and known to work :-)
</p>
TicketncohenSat, 03 Oct 2015 20:19:55 GMT
https://trac.sagemath.org/ticket/19340#comment:14
https://trac.sagemath.org/ticket/19340#comment:14
<blockquote class="citation">
<p>
Well, it's up to you whether you want to fix the broken code or just insert a few lines to call the code in flint that is faster and known to work :-)
</p>
</blockquote>
<p>
Thank you very much. Then I will do something else. I am trying to improve the database of strongly regular graphs and I don't want to interrupt that to write interface code.
</p>
<p>
Nathann
</p>
TicketgitSat, 03 Oct 2015 22:05:04 GMTcommit changed
https://trac.sagemath.org/ticket/19340#comment:15
https://trac.sagemath.org/ticket/19340#comment:15
<ul>
<li><strong>commit</strong>
changed from <em>247a2bb4dcec180227cd9fb9bcd331a27a289016</em> to <em>a5a4bbff80f2aed2a5b549d6b64f215967bc5ec7</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=a5a4bbff80f2aed2a5b549d6b64f215967bc5ec7"><span class="icon"></span>a5a4bbf</a></td><td><code>trac #19340: Pre-existing bug that hid several paley constructions</code>
</td></tr></table>
TicketvdelecroixSun, 04 Oct 2015 01:10:08 GMT
https://trac.sagemath.org/ticket/19340#comment:16
https://trac.sagemath.org/ticket/19340#comment:16
<p>
Hello,
</p>
<p>
Thanks Frederik for your suggestion (and your work in flint!). I pushed a branch at <code>u/vdelecroix/19340</code> with one commit which creates a function <code>hadamard_matrix_flint</code>. It could be called from the current <code>hadamard_matrix</code> code. It is faster. It is useful to have several implementations to check (as it was pointed out on a concrete example by Frederik)...
</p>
<p>
Vincent
</p>
TicketncohenSun, 04 Oct 2015 05:41:14 GMT
https://trac.sagemath.org/ticket/19340#comment:17
https://trac.sagemath.org/ticket/19340#comment:17
<p>
Yo,
</p>
<blockquote class="citation">
<p>
It is useful to have several implementations to check (as it was pointed out on a concrete example by Frederik)...
</p>
</blockquote>
<p>
Indeed. Please do that in <a class="closed ticket" href="https://trac.sagemath.org/ticket/19341" title="enhancement: Cleaning/Fix in combinat/matrices/hadamard_matrix (closed: fixed)">#19341</a>, though.
</p>
<p>
Nathann
</p>
TicketdimpaseSun, 04 Oct 2015 17:08:56 GMT
https://trac.sagemath.org/ticket/19340#comment:18
https://trac.sagemath.org/ticket/19340#comment:18
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/19340#comment:6" title="Comment 6">fredrik.johansson</a>:
</p>
<blockquote class="citation">
<p>
You might consider using the flint functions fmpz_mat_hadamard and fmpz_mat_is_hadamard, which should be faster.
</p>
<p>
Checking by default seems kind of useless since this is much slower than generating the matrix (generating a size 1000 Hadamard matrix takes 0.01 seconds and checking it takes 0.5 seconds),
</p>
</blockquote>
<p>
IMHO it says that your fmpz_mat_is_hadamard is far from optimal.
Indeed, you neither need to take the transpose nor to do full matrix multiplication.
</p>
<p>
As you fisrt check that the matrix is +/-1, you don't need to check that the diagonal entries of HH.T are all n.
So you only need to do scalar products of n(n-1)/2 rows...
</p>
<p>
(I understand that Hadamard matrices in FLINT don't do any serious job, they are there for testing or so; if one was after real speed there, you'd be using BLAS...)
</p>
Ticketfredrik.johanssonSun, 04 Oct 2015 17:22:14 GMT
https://trac.sagemath.org/ticket/19340#comment:19
https://trac.sagemath.org/ticket/19340#comment:19
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/19340#comment:18" title="Comment 18">dimpase</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/19340#comment:6" title="Comment 6">fredrik.johansson</a>:
</p>
<blockquote class="citation">
<p>
You might consider using the flint functions fmpz_mat_hadamard and fmpz_mat_is_hadamard, which should be faster.
</p>
<p>
Checking by default seems kind of useless since this is much slower than generating the matrix (generating a size 1000 Hadamard matrix takes 0.01 seconds and checking it takes 0.5 seconds),
</p>
</blockquote>
<p>
IMHO it says that your fmpz_mat_is_hadamard is far from optimal.
Indeed, you neither need to take the transpose nor to do full matrix multiplication.
</p>
<p>
As you fisrt check that the matrix is +/-1, you don't need to check that the diagonal entries of HH.T are all n.
So you only need to do scalar products of n(n-1)/2 rows...
</p>
</blockquote>
<p>
I didn't follow this. Are you saying that you can do the test in <code>O(n^2)</code> instead of <code>O(n^omega)</code>?
</p>
<blockquote class="citation">
<p>
(I understand that Hadamard matrices in FLINT don't do any serious job, they are there for testing or so; if one was after real speed there, you'd be using BLAS...)
</p>
</blockquote>
TicketdimpaseSun, 04 Oct 2015 17:39:55 GMT
https://trac.sagemath.org/ticket/19340#comment:20
https://trac.sagemath.org/ticket/19340#comment:20
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/19340#comment:19" title="Comment 19">fredrik.johansson</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/19340#comment:18" title="Comment 18">dimpase</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/19340#comment:6" title="Comment 6">fredrik.johansson</a>:
</p>
<blockquote class="citation">
<p>
You might consider using the flint functions fmpz_mat_hadamard and fmpz_mat_is_hadamard, which should be faster.
</p>
<p>
Checking by default seems kind of useless since this is much slower than generating the matrix (generating a size 1000 Hadamard matrix takes 0.01 seconds and checking it takes 0.5 seconds),
</p>
</blockquote>
<p>
IMHO it says that your fmpz_mat_is_hadamard is far from optimal.
Indeed, you neither need to take the transpose nor to do full matrix multiplication.
</p>
<p>
As you fisrt check that the matrix is +/-1, you don't need to check that the diagonal entries of HH.T are all n.
So you only need to do scalar products of n(n-1)/2 rows...
</p>
</blockquote>
<p>
I didn't follow this. Are you saying that you can do the test in <code>O(n^2)</code> instead of <code>O(n^omega)</code>?
</p>
</blockquote>
<p>
No, I am not saying that you can do the scalar product of two matrix rows in constant time. :-)
</p>
<p>
But at least you can replace
</p>
<pre class="wiki"> for (i = 0; i < n && result; i++)
for (j = 0; j < n && result; j++)
result = (*fmpz_mat_entry(C, i, j) == n * (i == j));
</pre><p>
with
</p>
<pre class="wiki"> for (i = 0; i < n && result; i++)
for (j = i+1; j < n && result; j++)
result = (*fmpz_mat_entry(C, i, j) == 0);
</pre><p>
another possible optimisation would be to have a dedicated function for matrix multiplication with the symmetric result (for <code>HH^T</code> is symmetric; other possible use would be to compute product of commuting symmetric matrices).
</p>
Ticketfredrik.johanssonSun, 04 Oct 2015 17:58:43 GMT
https://trac.sagemath.org/ticket/19340#comment:21
https://trac.sagemath.org/ticket/19340#comment:21
<p>
That part of the tests amounts to approximately 0.000% of the total running time.
</p>
<p>
In any case, a constant factor anywhere in the code doesn't fundamentally change the imbalance between testing and generating, as you suggested above.
</p>
<p>
The reason testing is far slower is not a factor 2 inefficiency (or even a factor 6 or whatever from not using BLAS) in the matrix multiplication, but the fact that constructing a Hadamard matrix is only <code>O(n^2)</code> while testing is <code>O(n^omega)</code>.
</p>
TicketdimpaseMon, 05 Oct 2015 15:20:49 GMT
https://trac.sagemath.org/ticket/19340#comment:22
https://trac.sagemath.org/ticket/19340#comment:22
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/19340#comment:21" title="Comment 21">fredrik.johansson</a>:
</p>
<blockquote class="citation">
<p>
That part of the tests amounts to approximately 0.000% of the total running time.
</p>
<p>
In any case, a constant factor anywhere in the code doesn't fundamentally change the imbalance between testing and generating, as you suggested above.
</p>
</blockquote>
<p>
as you imagine, I didn't profile this change. Still, it removes <code>O(n^2)</code> multiplications and comparisons. If you prefer to keep them in your code, it is up to you.
</p>
<blockquote class="citation">
<p>
The reason testing is far slower is not a factor 2 inefficiency (or even a factor 6 or whatever from not using BLAS) in the matrix multiplication, but the fact that constructing a Hadamard matrix is only <code>O(n^2)</code> while testing is <code>O(n^omega)</code>.
</p>
</blockquote>
<p>
On a good BLAS, the speed-up is a function of the number of cores available... Then, as we know, the devil is in the constants; if matrices were optimised for the task at hand, you would only have started to notice on 10000x10000 matrices, not on 1000x1000.
</p>
<p>
Anyhow, this is an academic discussion.
</p>
TicketdimpaseMon, 05 Oct 2015 18:15:58 GMT
https://trac.sagemath.org/ticket/19340#comment:23
https://trac.sagemath.org/ticket/19340#comment:23
<blockquote>
<p>
<code>Hadamard matrix, i.e. has its first row/column filles with +1.</code>
</p>
</blockquote>
<p>
should be <code>filled</code>
</p>
TicketgitMon, 05 Oct 2015 18:18:37 GMTcommit changed
https://trac.sagemath.org/ticket/19340#comment:24
https://trac.sagemath.org/ticket/19340#comment:24
<ul>
<li><strong>commit</strong>
changed from <em>a5a4bbff80f2aed2a5b549d6b64f215967bc5ec7</em> to <em>2553467c572dc462c4b514ba8e405fb928017928</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=2553467c572dc462c4b514ba8e405fb928017928"><span class="icon"></span>2553467</a></td><td><code>trac #19340: Typo</code>
</td></tr></table>
TicketdimpaseWed, 14 Oct 2015 17:33:54 GMT
https://trac.sagemath.org/ticket/19340#comment:25
https://trac.sagemath.org/ticket/19340#comment:25
<p>
my problem with this patch that it uses <code>is_prime</code> instead of <code>is_prime_power</code> to build Paley matrices. While the present implementation of Paley matrices needs primes, it's entirely unnecessary. Should I add a patch that fixes this here?
(or else, surely it's not a big deal to process errors from Paley constructions here rather than limit their inputs...)
</p>
TicketncohenWed, 14 Oct 2015 17:35:26 GMT
https://trac.sagemath.org/ticket/19340#comment:26
https://trac.sagemath.org/ticket/19340#comment:26
<p>
see <a class="closed ticket" href="https://trac.sagemath.org/ticket/19341" title="enhancement: Cleaning/Fix in combinat/matrices/hadamard_matrix (closed: fixed)">#19341</a>.
</p>
TicketdimpaseWed, 14 Oct 2015 17:43:36 GMTstatus changed
https://trac.sagemath.org/ticket/19340#comment:27
https://trac.sagemath.org/ticket/19340#comment:27
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/19340#comment:26" title="Comment 26">ncohen</a>:
</p>
<blockquote class="citation">
<p>
see <a class="closed ticket" href="https://trac.sagemath.org/ticket/19341" title="enhancement: Cleaning/Fix in combinat/matrices/hadamard_matrix (closed: fixed)">#19341</a>.
</p>
</blockquote>
<p>
OK!
</p>
TicketncohenWed, 14 Oct 2015 17:49:08 GMT
https://trac.sagemath.org/ticket/19340#comment:28
https://trac.sagemath.org/ticket/19340#comment:28
<p>
Thanks,
</p>
<p>
Nathann
</p>
<p>
P.S.: What about the... You know... Reviewer field?
</p>
TicketdimpaseWed, 14 Oct 2015 18:11:59 GMTreviewer set
https://trac.sagemath.org/ticket/19340#comment:29
https://trac.sagemath.org/ticket/19340#comment:29
<ul>
<li><strong>reviewer</strong>
set to <em>Dima Pasechnik</em>
</li>
</ul>
TicketvbraunFri, 16 Oct 2015 08:22:19 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/19340#comment:30
https://trac.sagemath.org/ticket/19340#comment:30
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>branch</strong>
changed from <em>u/ncohen/19340</em> to <em>2553467c572dc462c4b514ba8e405fb928017928</em>
</li>
</ul>
Ticket