Skip to content
This repository was archived by the owner on Jul 24, 2024. It is now read-only.

[Merged by Bors] - feat(data/matrix/basic): add conj_transpose#8291

Closed
l534zhan wants to merge 33 commits intomasterfrom
conj_trans_lz
Closed

[Merged by Bors] - feat(data/matrix/basic): add conj_transpose#8291
l534zhan wants to merge 33 commits intomasterfrom
conj_trans_lz

Conversation

@l534zhan
Copy link
Copy Markdown
Collaborator

@l534zhan l534zhan commented Jul 13, 2021

As requested by Eric Wieser, I pulled one single change of #8289 out into a new PR. As such, this PR will not block anything in #8289.


Open in Gitpod

@l534zhan l534zhan added awaiting-review The author would like community review of the PR easy < 20s of review time. See the lifecycle page for guidelines. labels Jul 13, 2021
@l534zhan l534zhan requested a review from eric-wieser July 13, 2021 11:25
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
l534zhan and others added 3 commits July 13, 2021 13:01
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
@eric-wieser
Copy link
Copy Markdown
Member

Every change of #8291 is also updated into #8289.
If #8289 is approved first, then this PR will be redundant.

I don't recommend this strategy - please focus on this PR first. Large PRs are harder to review, especially if they do lots of unrelated things.

@eric-wieser eric-wieser removed the easy < 20s of review time. See the lifecycle page for guidelines. label Jul 13, 2021
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
@l534zhan
Copy link
Copy Markdown
Collaborator Author

l534zhan commented Jul 13, 2021

@eric-wieser, I thought what you mentioned are several blocks of lemmas. As they are just single lemmas, I think I can do them today.

@eric-wieser
Copy link
Copy Markdown
Member

That's great news! Having looked again, the two update_* lemmas need the pi.has_star instance that you had difficulty with before, so let's not bother with those two in this PR.

@l534zhan
Copy link
Copy Markdown
Collaborator Author

l534zhan commented Jul 13, 2021

The other two has been added! @eric-wieser
I think the solution for the other PR is, if some new definitions seem should be in new files, then just remove them. I didn't give any lemma to one or two defs anyway.

Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Copy link
Copy Markdown
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 🎉

@Vierkantor, would you mind giving this a final lookover? Are you happy with the addition to the authors list?

@eric-wieser eric-wieser requested a review from Vierkantor July 13, 2021 18:24
@l534zhan
Copy link
Copy Markdown
Collaborator Author

I have another pending PR #8289 contributing to this file as well.

@eric-wieser
Copy link
Copy Markdown
Member

That's fine, git will handle unifying the two for us. Once this one is merged into master, I can help you merge the changes into your other PR.

@Vierkantor
Copy link
Copy Markdown
Collaborator

Are you happy with the addition to the authors list?

Yes! I care very little about determining who counts as an "author" of a file. If you contribute to a file and would like credit for it, go for it, I would say.

Copy link
Copy Markdown
Collaborator

@Vierkantor Vierkantor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

Copy link
Copy Markdown
Collaborator

@Vierkantor Vierkantor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors r+

@github-actions github-actions bot added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Jul 13, 2021
bors bot pushed a commit that referenced this pull request Jul 13, 2021
As requested by Eric Wieser, I pulled one single change of #8289 out into a new PR. As such, this PR will not block anything in #8289.



Co-authored-by: l534zhan <84618936+l534zhan@users.noreply.github.com>
@bors
Copy link
Copy Markdown

bors bot commented Jul 13, 2021

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Jul 13, 2021
As requested by Eric Wieser, I pulled one single change of #8289 out into a new PR. As such, this PR will not block anything in #8289.



Co-authored-by: l534zhan <84618936+l534zhan@users.noreply.github.com>
@l534zhan
Copy link
Copy Markdown
Collaborator Author

@eric-wieser Eric, an issue unrelated to the file changed seems to have arisen.

@Vierkantor
Copy link
Copy Markdown
Collaborator

Don't worry. Bors should automatically figure out which commit is causing the issue, and merge all the correct commits.

@l534zhan
Copy link
Copy Markdown
Collaborator Author

@eric-wieser

@[simp] lemma conj_transpose_one [decidable_eq n] [semiring α] [star_ring α]:
  (1 : matrix n n α)ᴴ = 1 :=
by simp [conj_transpose]

This is very amazing to me. How can I know which lemmas simp used?

@eric-wieser
Copy link
Copy Markdown
Member

You can replace simp with simp? and it will tell you what lemmas it used

@bors
Copy link
Copy Markdown

bors bot commented Jul 13, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(data/matrix/basic): add conj_transpose [Merged by Bors] - feat(data/matrix/basic): add conj_transpose Jul 13, 2021
@bors bors bot closed this Jul 13, 2021
@bors bors bot deleted the conj_trans_lz branch July 13, 2021 21:31
bors bot pushed a commit that referenced this pull request Jul 15, 2021
…8303)

This also moves some imports around to make the star operator on vectors available in matrix/basic.lean

This is a follow up to #8291
b-mehta pushed a commit that referenced this pull request Jul 20, 2021
As requested by Eric Wieser, I pulled one single change of #8289 out into a new PR. As such, this PR will not block anything in #8289.



Co-authored-by: l534zhan <84618936+l534zhan@users.noreply.github.com>
b-mehta pushed a commit that referenced this pull request Jul 20, 2021
…8303)

This also moves some imports around to make the star operator on vectors available in matrix/basic.lean

This is a follow up to #8291
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants