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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - feat(linear_algebra/matrix/determinant): generalize det_fin_zero to det_eq_one_of_is_empty #8387

Closed
wants to merge 6 commits into from
Closed

[Merged by Bors] - feat(linear_algebra/matrix/determinant): generalize det_fin_zero to det_eq_one_of_is_empty #8387

wants to merge 6 commits into from

Conversation

eric-wieser
Copy link
Member

Also golfs a nearby proof

Open in Gitpod

@eric-wieser
feat(linear_algebra/matrix/determinant): generalize det_fin_zero to d…
2731d80 
…et_eq_one_of_is_empty

Also golfs a nearby proof
@eric-wieser eric-wieser added the awaiting-review The author would like community review of the PR label Jul 21, 2021
Vierkantor
Vierkantor reviewed Jul 22, 2021
View reviewed changes
Copy link
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.

Have you tried showing det_eq_one_of_is_empty directly? Ideally it would be at least as easy as det_eq_one_of_card_eq_zero, so if it is more difficult that would suggest that there are some missing lemmas.

The generalization looks good to me otherwise.

@eric-wieser
Copy link
Member Author

We were missing the unique (perm a) instance needed for my proof. I've added it now, and will wait for a cache.

Vierkantor
Vierkantor approved these changes Jul 22, 2021
View reviewed changes
Copy link
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, thanks for fixing all the dependencies!

bors r+

@github-actions github-actions bot added the ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) label Jul 22, 2021
bors bot pushed a commit that referenced this pull request Jul 22, 2021
@eric-wieser
feat(linear_algebra/matrix/determinant): generalize det_fin_zero to d…
0cbc6f3 
…et_eq_one_of_is_empty (#8387)

Also golfs a nearby proof
@github-actions github-actions bot removed the awaiting-review The author would like community review of the PR label Jul 22, 2021
@bors
Copy link

bors bot commented Jul 22, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(linear_algebra/matrix/determinant): generalize det_fin_zero to det_eq_one_of_is_empty [Merged by Bors] - feat(linear_algebra/matrix/determinant): generalize det_fin_zero to det_eq_one_of_is_empty Jul 22, 2021
@bors bors bot closed this Jul 22, 2021
@bors bors bot deleted the eric-wieser/matrix.det-of-is_empty branch July 22, 2021 23:14
pechersky added a commit that referenced this pull request Jul 25, 2021
@pechersky
Remove duplicate unique perm instance
ac1353b 
PR #8387 already gave this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Vierkantor Vierkantor Vierkantor approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@eric-wieser @Vierkantor