#19019 closed defect (fixed)
Very careless typoes in strongly_regular_db
Reported by:  ncohen  Owned by:  

Priority:  major  Milestone:  sage6.9 
Component:  graph theory  Keywords:  
Cc:  dimpase  Merged in:  
Authors:  Nathann Cohen  Reviewers:  Dima Pasechnik 
Report Upstream:  N/A  Work issues:  
Branch:  cb0b3fb (Commits, GitHub, GitLab)  Commit:  
Dependencies:  #19018  Stopgaps: 
Description
This branch fixes very bad typoes in that file, that made several constructions useless or broken. Mostly missing characters (emacs macros...)
Nathann
Change History (30)
comment:1 Changed 6 years ago by
 Branch set to u/ncohen/19019
 Status changed from new to needs_review
comment:2 Changed 6 years ago by
 Commit set to 05b2fa885325eab312bd8380f22fe8690b14afae
comment:3 followup: ↓ 5 Changed 6 years ago by
Not relating to just this patch, but why functions like SRG_100_44_18_20()
, SRG_100_45_20_20()
etc. instead of something like SRG_from_db(100, 44, 18, 20)
? There might be some rationale explained in some other ticket.
comment:4 Changed 6 years ago by
 Commit changed from 05b2fa885325eab312bd8380f22fe8690b14afae to 4abbce4ea5a32d1b82eae7de85fa3dc5f695e5da
comment:5 in reply to: ↑ 3 Changed 6 years ago by
Not relating to just this patch, but why functions like
SRG_100_44_18_20()
,SRG_100_45_20_20()
etc. instead of something likeSRG_from_db(100, 44, 18, 20)
? There might be some rationale explained in some other ticket.
I do not understand what you mean, and I do not want the conversation to happen on this unrelated ticket. Please send me an email or write to sagedevel.
comment:6 Changed 6 years ago by
 Commit changed from 4abbce4ea5a32d1b82eae7de85fa3dc5f695e5da to c5e24be70ec523d6c91af7c7a0664cce9459e2cf
Branch pushed to git repo; I updated commit sha1. New commits:
c5e24be  trac #19019: Missing space

comment:7 Changed 6 years ago by
 Commit changed from c5e24be70ec523d6c91af7c7a0664cce9459e2cf to 3f76966eaf187f22485471c04f6227f22a024efe
comment:8 Changed 6 years ago by
 Commit changed from 3f76966eaf187f22485471c04f6227f22a024efe to 19b40d996b2770b58bfefbea5bb8d3c0bded6047
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
d66f0b6  trac #19018: Specify which graph is built

1cae2dc  trac #19028: Table 8.1

7045124  trac #19018: Merged with 6.9.beta2

99fa6ce  trac #19019: Very careless typoes in strongly_regular_db

cb071e2  trac #19019: Missing space

19b40d9  trac #19019: Sort the list of SRGs

comment:9 Changed 6 years ago by
 Branch changed from u/ncohen/19019 to public/19019
 Commit changed from 19b40d996b2770b58bfefbea5bb8d3c0bded6047 to 778556f1afddb74933575180e36018f0608fba2a
 Status changed from needs_review to positive_review
rebased over the latest developments on #19018. LGTM
comment:10 Changed 6 years ago by
Volker will reject this if you don't add your name to Reviewersfield.
comment:11 followup: ↓ 12 Changed 6 years ago by
 Commit changed from 778556f1afddb74933575180e36018f0608fba2a to 323416f9a5a46f1cc6765ef54906938353eee856
 Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
323416f  added a doctest

comment:12 in reply to: ↑ 11 Changed 6 years ago by
 Status changed from needs_review to positive_review
comment:13 Changed 6 years ago by
What you did is not a rebase, nor a merge. You apparently cherrypicked your commit on top of that branch (note that its hash changed). As a result your commit appears twice in the history.
Please remove this latest commit: it is not because the branch are not exactly the same that they do not merge trivially. I don't think that your last commit from #19018 caused the slightest problem with this branch.
Nathann
comment:14 Changed 6 years ago by
 Status changed from positive_review to needs_work
Reviewer name missing
comment:15 Changed 6 years ago by
 Reviewers set to Dima Pasechnik
 Status changed from needs_work to positive_review
I cannot find the extra commit in question.
comment:16 Changed 6 years ago by
It is the last commit of this branch.
comment:17 Changed 6 years ago by
In the local branch I do not have 2 copies of this commit. Naturally, if I were to remove it and push the result then it would not be there at all.
comment:18 Changed 6 years ago by
In the local branch you have this commit. In the branch of #19018 you have another copy of it. Because you cherrypicked it, when the two branches will be merged you will have two copies of it.
This commit belongs to the other branch, not to that one. If you remove it from this branch, it will be in Sage when #19018 will be merged.
You should have merged the branches, not cherrypicked the commit.
Nathann
comment:19 Changed 6 years ago by
 Commit changed from 323416f9a5a46f1cc6765ef54906938353eee856 to cb0b3fbd221fb01c0e392d141ba50427b0a0294f
 Status changed from positive_review to needs_review
comment:20 Changed 6 years ago by
please review these git things. I'll be offline for the coming 10 hours.
comment:21 Changed 6 years ago by
 Status changed from needs_review to positive_review
Looks good, thanks.
Nathann
comment:22 Changed 6 years ago by
 Branch changed from public/19019 to cb0b3fbd221fb01c0e392d141ba50427b0a0294f
 Resolution set to fixed
 Status changed from positive_review to closed
comment:23 followup: ↓ 24 Changed 6 years ago by
 Commit cb0b3fbd221fb01c0e392d141ba50427b0a0294f deleted
I overlooked
+ When `\epsilon\in\{1,+1\}`, we say that `M` is a `(n,\epsilon)RSHCD` if + `M` is a regular symmetric Hadamard matrix with constant diagonal + `\delta\in\{1,+1\}` and row values all equal to `\delta \epsilon + \sqrt(n)`. For more information, see [HX10]_ or 10.5.1 in + [BH12]_.
 what are "row values"? row sums?
 the definition of RSHCD here does not match one in [BH12], where the diagonal entries are all 1, and (n,e)RSHCD has row sums e\sqrt{n}.
 it should be \sqrt{n}, not \sqrt(n)
And another thing  RSHCD should be mentioned in the module description in the beginning of the corr. file.
comment:24 in reply to: ↑ 23 ; followups: ↓ 25 ↓ 27 Changed 6 years ago by
 what are "row values"? row sums?
Arg... Yes. Row sums.
 the definition of RSHCD here does not match one in [BH12], where the diagonal entries are all 1, and (n,e)RSHCD has row sums e\sqrt{n}.
So what do we do? With two papers that say contradictory things?
 it should be \sqrt{n}, not \sqrt(n)
Right.
And another thing  RSHCD should be mentioned in the module description in the beginning of the corr. file.
I don't understand.
Nathann
comment:25 in reply to: ↑ 24 ; followup: ↓ 26 Changed 6 years ago by
Replying to ncohen:
So what do we do? With two papers that say contradictory things?
IMO select one and (almost) always document these cases. See chain_polynomial() on posets: "Warning: This is not what has been called the chain polynomial in [St1986]. The latter is  ".
comment:26 in reply to: ↑ 25 Changed 6 years ago by
IMO select one and (almost) always document these cases. See chain_polynomial() on posets: "Warning: This is not what has been called the chain polynomial in [St1986]. The latter is  ".
Jori, this was a rethoric question. The two definitions agree with each other.
comment:27 in reply to: ↑ 24 ; followup: ↓ 28 Changed 6 years ago by
Replying to ncohen:
 the definition of RSHCD here does not match one in [BH12], where the diagonal entries are all 1, and (n,e)RSHCD has row sums e\sqrt{n}.
So what do we do? With two papers that say contradictory things?
I was comparing your definition with the one from http://homepages.cwi.nl/~aeb/math/ipm/ipm.pdf and your definition is more complicated. I did not check the other one.
And another thing  RSHCD should be mentioned in the module description in the beginning of the corr. file.
I don't understand.
If look at combinat/matrices/hadamard_matrix.py there is a paragraph saying
The module below implements the Paley constructions (see for example [Hora]_) and the Sylvester construction. It also allows you to pull a Hadamard matrix from the database at [HadaSloa]_.
But there is no word about RSHCDs. Should it instead get a list of methods, etc, as more modern Sage modules have?
As well, you're not in the list of authors.
As I am gearing up for skewHadamard matrices, it can be done on the corresponding ticket.
comment:28 in reply to: ↑ 27 ; followup: ↓ 29 Changed 6 years ago by
I was comparing your definition with the one from http://homepages.cwi.nl/~aeb/math/ipm/ipm.pdf and your definition is more complicated. I did not check the other one.
I don't see how. In the paper you give the matrix is requested to have 1 on the diagonal, that's the only difference I see.
If look at combinat/matrices/hadamard_matrix.py there is a paragraph saying But there is no word about RSHCDs.
Oh, right. I'll do that.
Should it instead get a list of methods, etc, as more modern Sage modules have?
Probably. I am working on a patch related to index of functions at the moment, though. Something to make building a thematic index easier.
As well, you're not in the list of authors.
How I hate this "I am the one who did it" bullshit...
As I am gearing up for skewHadamard matrices, it can be done on the corresponding ticket.
Skewhadamard? *sigh*.... Someday we will have all kind of designs in Sage. But today is not this day.
Nathann
comment:29 in reply to: ↑ 28 ; followup: ↓ 30 Changed 6 years ago by
Replying to ncohen:
I was comparing your definition with the one from http://homepages.cwi.nl/~aeb/math/ipm/ipm.pdf and your definition is more complicated. I did not check the other one.
I don't see how. In the paper you give the matrix is requested to have 1 on the diagonal, that's the only difference I see.
yes, this is the difference that makes your definition different. I suppose this is very close to the original one, as all is needed is multiplication by 1 to convert one into the other. But still not the same...
As well, you're not in the list of authors.
How I hate this "I am the one who did it" bullshit...
well, if only for uniformity...
As I am gearing up for skewHadamard matrices, it can be done on the corresponding ticket.
Skewhadamard? *sigh*.... Someday we will have all kind of designs in Sage. But today is not this day.
we need them to build "skewhad*" and your humble servant srg's, as I already mentioned.
comment:30 in reply to: ↑ 29 Changed 6 years ago by
yes, this is the difference that makes your definition different. I suppose this is very close to the original one, as all is needed is multiplication by 1 to convert one into the other. But still not the same...
For all practical purposes it is the same. One says that the diagonal has to be 1, the other says that it can be 1 but adapts the value of row sums. If you get one which does not have a 1 on the diagonal, then your_matrix
does the job, and all results of existence/nonexistence apply.
Nathann
Branch pushed to git repo; I updated commit sha1. New commits:
trac #19018: More SRGs using Regular Symmetric Hadamard matric with Constant Diagonal
trac #19019: Very careless typoes in strongly_regular_db