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

Very careless typoes in strongly_regular_db #19019

Closed
nathanncohen mannequin opened this issue Aug 12, 2015 · 41 comments
Closed

Very careless typoes in strongly_regular_db #19019

nathanncohen mannequin opened this issue Aug 12, 2015 · 41 comments

Comments

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Aug 12, 2015

This branch fixes very bad typoes in that file, that made several constructions useless or broken. Mostly missing characters (emacs macros...)

Nathann

Depends on #19018

CC: @dimpase

Component: graph theory

Author: Nathann Cohen

Branch: cb0b3fb

Reviewer: Dima Pasechnik

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

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

nathanncohen mannequin commented Aug 12, 2015

Branch: u/ncohen/19019

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

sagetrac-git mannequin commented Aug 12, 2015

Commit: 05b2fa8

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 12, 2015

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

a414c36trac #19018: More SRGs using Regular Symmetric Hadamard matric with Constant Diagonal
05b2fa8trac #19019: Very careless typoes in strongly_regular_db

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Aug 13, 2015

comment:3

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 13, 2015

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

c430b4btrac #19018: Link the new constructions with graphs.strongly_regular_graph
0e533bbtrac #19018: Bibliography
4abbce4trac #19019: Very careless typoes in strongly_regular_db

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 13, 2015

Changed commit from 05b2fa8 to 4abbce4

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 13, 2015

comment:5

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.

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 sage-devel.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 13, 2015

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

c5e24betrac #19019: Missing space

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 13, 2015

Changed commit from 4abbce4 to c5e24be

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 13, 2015

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

6893ad1trac #19018: Typo
e18bee7trac #19019: Very careless typoes in strongly_regular_db
3f76966trac #19019: Missing space

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 13, 2015

Changed commit from c5e24be to 3f76966

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 14, 2015

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

d66f0b6trac #19018: Specify which graph is built
1cae2dctrac #19028: Table 8.1
7045124trac #19018: Merged with 6.9.beta2
99fa6cetrac #19019: Very careless typoes in strongly_regular_db
cb071e2trac #19019: Missing space
19b40d9trac #19019: Sort the list of SRGs

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 14, 2015

Changed commit from 3f76966 to 19b40d9

@dimpase
Copy link
Member

dimpase commented Aug 14, 2015

Changed branch from u/ncohen/19019 to public/19019

@dimpase
Copy link
Member

dimpase commented Aug 14, 2015

comment:9

rebased over the latest developments on #19018. LGTM

@dimpase
Copy link
Member

dimpase commented Aug 14, 2015

Changed commit from 19b40d9 to 778556f

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Aug 14, 2015

comment:10

Volker will reject this if you don't add your name to Reviewers-field.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 16, 2015

Changed commit from 778556f to 323416f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 16, 2015

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

323416fadded a doctest

@dimpase
Copy link
Member

dimpase commented Aug 16, 2015

comment:12

Replying to @sagetrac-git:

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

323416fadded a doctest

rebased over the latest #19018 branch

@dimpase
Copy link
Member

dimpase commented Aug 16, 2015

comment:15

I cannot find the extra commit in question.

@dimpase
Copy link
Member

dimpase commented Aug 16, 2015

Reviewer: Dima Pasechnik

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 16, 2015

comment:16

It is the last commit of this branch.

@dimpase
Copy link
Member

dimpase commented Aug 16, 2015

comment:17

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.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 16, 2015

comment:18

In the local branch you have this commit. In the branch of #19018 you have another copy of it. Because you cherry-picked 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 cherry-picked the commit.

Nathann

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 16, 2015

Changed commit from 323416f to cb0b3fb

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 16, 2015

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:

6ed716dadded a doctest
cb0b3fbMerge branch 'reg_symm_hadamard' into t19019

@dimpase
Copy link
Member

dimpase commented Aug 16, 2015

comment:20

please review these git things. I'll be offline for the coming 10 hours.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 16, 2015

comment:21

Looks good, thanks.

Nathann

@vbraun
Copy link
Member

vbraun commented Aug 19, 2015

Changed branch from public/19019 to cb0b3fb

@dimpase
Copy link
Member

dimpase commented Aug 19, 2015

comment:23

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.

@dimpase
Copy link
Member

dimpase commented Aug 19, 2015

Changed commit from cb0b3fb to none

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 19, 2015

comment:24
  • 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

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Aug 19, 2015

comment:25

Replying to @nathanncohen:

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 - -".

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 19, 2015

comment:26

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.

@dimpase
Copy link
Member

dimpase commented Aug 19, 2015

comment:27

Replying to @nathanncohen:

  • 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 skew-Hadamard matrices, it can be done on the corresponding ticket.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 19, 2015

comment:28

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 skew-Hadamard matrices, it can be done on the corresponding ticket.

Skew-hadamard? sigh.... Someday we will have all kind of designs in Sage. But today is not this day.

Nathann

@dimpase
Copy link
Member

dimpase commented Aug 19, 2015

comment:29

Replying to @nathanncohen:

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 skew-Hadamard matrices, it can be done on the corresponding ticket.

Skew-hadamard? 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.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 19, 2015

comment:30

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/non-existence apply.

Nathann

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