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

[Merged by Bors] - feat(data/nat/choose/multinomial): add multinomial theorem #16317

Closed
wants to merge 45 commits into from

Conversation

pimotte
Copy link
Collaborator

@pimotte pimotte commented Aug 30, 2022

Proof by induction on the number of summands. finset.sym is used to sum over. It means we have access to the finset tooling to rewrite it, and it's one of the more direct ways of expressing it.


Open in Gitpod

@pimotte pimotte added awaiting-review The author would like community review of the PR awaiting-CI The author would like to see what CI has to say before doing more work. labels Aug 30, 2022
@mathlib-dependent-issues-bot mathlib-dependent-issues-bot added the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Aug 30, 2022
@github-actions github-actions bot removed the awaiting-CI The author would like to see what CI has to say before doing more work. label Aug 30, 2022
@pimotte pimotte added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Sep 12, 2022
@pimotte
Copy link
Collaborator Author

pimotte commented Sep 12, 2022

Due to changes in #16316 this will need some changes and likely another PR dependency

@pimotte pimotte added awaiting-review The author would like community review of the PR awaiting-CI The author would like to see what CI has to say before doing more work. and removed awaiting-author A reviewer has asked the author a question or requested changes labels Sep 12, 2022
@alreadydone alreadydone added the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Oct 13, 2022
@@ -87,9 +99,9 @@ by simpa [finset.sum_pair hab, finset.prod_pair hab] using multinomial_spec {a,
by simp [multinomial_insert_one {b} f (finset.not_mem_singleton.mpr h) h₁]

lemma binomial_succ_succ [decidable_eq α] (h : a ≠ b) :
Copy link
Member

Choose a reason for hiding this comment

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

Should this name be changed? There is no binomial nor succ in the statement. I see something similar also below.

Copy link
Collaborator

Choose a reason for hiding this comment

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

There are six lemmas with binomial in this file, and I think they are intended to establish a naming convention that binomial refers to multinomial {a, b} with a ≠ b (or maybe more generally finsets of cardinality two, but they don't appear in this file).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think multinomial_pair is better in place of binomial? If so I can rename all in this PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @riccardobrasca Do you think I should do the rename? You other comments have been marked resolved, and the only change in this PR since your last review was extracting a lemma.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I forgot about this PR. It binomial is intended to be a name convention than it's OK. Maybe we can write it somewhere in the doc (if it's not already present)? Otherwise LGTM, thanks!

Copy link
Collaborator

@alreadydone alreadydone Oct 24, 2022

Choose a reason for hiding this comment

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

It turns out binomial traces back to the original code written by @kmill, so I think at least one maintainer is OK with the names; I just updated the section docstring to state this naming convention. @pimotte could you just reply bors r+ to get this PR merged? Thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done! Thanks for finishing this up:)

src/data/nat/choose/multinomial.lean Show resolved Hide resolved
src/data/nat/choose/multinomial.lean Show resolved Hide resolved
src/data/nat/choose/multinomial.lean Show resolved Hide resolved
@mathlib-dependent-issues-bot mathlib-dependent-issues-bot removed the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Oct 15, 2022
bors bot pushed a commit that referenced this pull request Oct 22, 2022
…17108)

elaboration of has_sum_single took 17.9s -> 1.37s

The timeout happens in an unrelated branch #16317 ([CI log](https://github.com/leanprover-community/mathlib/actions/runs/3294947441/jobs/5432990411)) but not yet in bors batches apparently.
@riccardobrasca
Copy link
Member

bors d+

@bors
Copy link

bors bot commented Oct 23, 2022

✌️ pimotte can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@github-actions github-actions bot added delegated The PR author may merge after reviewing final suggestions. and removed awaiting-review The author would like community review of the PR labels Oct 23, 2022
@pimotte
Copy link
Collaborator Author

pimotte commented Oct 24, 2022

bors r+

bors bot pushed a commit that referenced this pull request Oct 24, 2022
Proof by induction on the number of summands. finset.sym is used to sum over. It means we have access to the finset tooling to rewrite it, and it's one of the more direct ways of expressing it.

Co-authored-by: Junyan Xu <junyanxu.math@gmail.com>



Co-authored-by: Pim Otte <potte@quintor.nl>
Co-authored-by: Junyan Xu <junyanxumath@gmail.com>
Co-authored-by: Junyan Xu <junyanxu.math@gmail.com>
@bors
Copy link

bors bot commented Oct 24, 2022

This PR was included in a batch that was canceled, it will be automatically retried

bors bot pushed a commit that referenced this pull request Oct 24, 2022
Proof by induction on the number of summands. finset.sym is used to sum over. It means we have access to the finset tooling to rewrite it, and it's one of the more direct ways of expressing it.

Co-authored-by: Junyan Xu <junyanxu.math@gmail.com>



Co-authored-by: Pim Otte <potte@quintor.nl>
Co-authored-by: Junyan Xu <junyanxumath@gmail.com>
Co-authored-by: Junyan Xu <junyanxu.math@gmail.com>
@bors
Copy link

bors bot commented Oct 24, 2022

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Oct 24, 2022
Proof by induction on the number of summands. finset.sym is used to sum over. It means we have access to the finset tooling to rewrite it, and it's one of the more direct ways of expressing it.



Co-authored-by: Pim Otte <potte@quintor.nl>
Co-authored-by: Junyan Xu <junyanxumath@gmail.com>
Co-authored-by: Junyan Xu <junyanxu.math@gmail.com>
@bors
Copy link

bors bot commented Oct 24, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(data/nat/choose/multinomial): add multinomial theorem [Merged by Bors] - feat(data/nat/choose/multinomial): add multinomial theorem Oct 24, 2022
@bors bors bot closed this Oct 24, 2022
@bors bors bot deleted the multinomial-theorem-1 branch October 24, 2022 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated The PR author may merge after reviewing final suggestions. t-algebra Algebra (groups, rings, fields etc) t-combinatorics Combinatorics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants