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: ∑ x ∈ s, f x to replace ∑ x in s, f x in the future #6795

Closed
wants to merge 11 commits into from

Conversation

kmill
Copy link
Contributor

@kmill kmill commented Aug 25, 2023

Adds new syntax for sum/product big operators for ∑ x ∈ s, f x. The set s can either be a Finset or a Set with a Fintype instance, in which case it is equivalent to ∑ x ∈ s.toFinset, f x.

Also supports ∑ (x ∈ s) (y ∈ t), f x y for Finset.sum (s ×ˢ t) (fun ⟨x, y⟩ ↦ f x y). Note that these are not dependent products at the moment.

Adds with clauses, so for example ∑ (x ∈ Finset.range 5) (y ∈ Finset.range 5) with x < y, x * y, which inserts a Finset.filter over the domain set.

Supports pattern matching in the variable position. This is by creating an experimental version of extBinder that uses term:max instead of binderIdent.

The new ∑ x ∈ s, f x notation is used in Algebra.BigOperators.Basic for illustration, but the old ∑ x in s, f x still works for backwards compatibility.

Zulip threads here and here


Open in Gitpod

@kmill kmill added the awaiting-review The author would like community review of the PR label Aug 25, 2023
@kmill kmill 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 Aug 26, 2023
@kmill kmill added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Aug 26, 2023
| `(finset% $t) => precheck t
| _ => Elab.throwUnsupportedSyntax

-- TODO: contribute this modification back to `extBinder`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The changes to the extBinder syntax are in https://github.com/leanprover/std4/pull/234/files and in the meantime it's given as new syntax as bigOpBinder here.

- `∑ x ∈ s, f x` is notation for `Finset.sum s f`. It is the sum of `f x`,
where `x` ranges over the finite set `s` (either a `Finset` or a `Set` with a `Fintype` instance).
- `∑ x ∈ s with p x, f x` is notation for `Finset.sum (Finset.filter p s) f`.
- `∑ (x ∈ s) (y ∈ t), f x y` is notation for `Finset.sum (s ×ˢ t) (fun ⟨x, y⟩ ↦ f x y)`.
Copy link
Member

Choose a reason for hiding this comment

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

Is it a valid alternative to elaborate this into a nested sum? That would automatically handle the case of t depending on x, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The thought was that if you wanted a nested sum then ∑ (x ∈ s), ∑ (y ∈ t), f x y is available. Yaël and Bhavik were telling me about how this sort of behavior is helpful for related notations such as for expectation, so it might be worth being consistent here.

A downside though is that this PR is not supporting dependent products, i.e., you can't do ∑ (x ∈ s) (y ∈ t x), f x y, and it's not very clear to me how to support them in a low-confusion way.

@jcommelin jcommelin 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 14, 2023
Copy link
Collaborator

@YaelDillies YaelDillies left a comment

Choose a reason for hiding this comment

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

I really want this notation to land. I am using it all over the place in LeanAPAP. The elaborator isn't perfect yet (no dependent products, unspecified interaction between nested and filtered products...) but it would already be a great step forward.

Comment on lines 103 to 119
/-- `finset% t` elaborates `t` as a `Finset`.
If `t` is a `Set`, then inserts `Set.toFinset`.
Does not make use of the expected type; useful for big operators over finsets.
```
#check finset% Finset.range 2 -- Finset Nat
#check finset% (Set.univ : Set Bool) -- Finset Bool
```
-/
elab (name := finsetStx) "finset% " t:term : term => do
let u ← mkFreshLevelMVar
let ty ← mkFreshExprMVar (mkSort (.succ u))
let x ← Elab.Term.elabTerm t (mkApp (.const ``Finset [u]) ty)
let xty ← whnfR (← inferType x)
if xty.isAppOfArity ``Set 1 then
Elab.Term.elabAppArgs (.const ``Set.toFinset [u]) #[] #[.expr x] none false false
else
return x
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be moved to an earlier file?

else
let ss ← withNaryArg 3 <| delab
`(∏ $(.mk i):ident in $ss, $body)
`(∏ $(.mk i):ident $ss, $body)

/-- Delaborator for `Finset.prod`. The `pp.piBinderTypes` option controls whether
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/-- Delaborator for `Finset.prod`. The `pp.piBinderTypes` option controls whether
/-- Delaborator for `Finset.sum`. The `pp.piBinderTypes` option controls whether

@bors bors bot changed the base branch from master to ScottCarnahan/BinomialRing2 September 17, 2023 03:26
@semorrison semorrison changed the base branch from ScottCarnahan/BinomialRing2 to master September 17, 2023 12:11
@kmill
Copy link
Contributor Author

kmill commented Sep 18, 2023

@YaelDillies I'm not so sure about this being the sum/prod notation, but I do support having some variant notation that constructs these (dependent) products for the domain.

In #7227 I've been working on a more flexible binder notation to upgrade all notation3 binders to have pattern matching etc. (and you can define your own binders!). Part of it is that ideally sum/prod notation uses it as well for consistency, which that PR succeeds in doing.

Potentially the variant notation could be a new binder type. Just making one up quick for sake of illustration, ∑ join% (x ∈ s) (y ∈ t), f x y could work with this system.

@YaelDillies
Copy link
Collaborator

What I really care about is the \sum ... with ..., ... notation. If that makes it easier to get in, would you mind opening a PR with just this change. Then I can copy it in LeanAPAP, check it works there, and subsequently send your PR on the merge queue.

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Sep 24, 2023
@YaelDillies YaelDillies added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes merge-conflict The PR has a merge conflict with master, and needs manual merging. labels Mar 22, 2024
@YaelDillies YaelDillies added the t-meta Tactics, attributes or user commands label Mar 22, 2024
Copy link
Collaborator

@YaelDillies YaelDillies left a comment

Choose a reason for hiding this comment

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

I think this is great overall and deserves to be merged even if a few details still need ironing out. We need to try out the new syntaxes to eg decide what the correct behavior for nested sums is (I think the one in this PR is already correct, but who knows).

maintainer merge

Copy link

🚀 Pull request has been placed on the maintainer queue by YaelDillies.

@Ruben-VandeVelde Ruben-VandeVelde changed the title feat: ∑ x ∈ s, f x instead of ∑ x in s, f x feat: ∑ x ∈ s, f x to replace ∑ x in s, f x in the future Mar 22, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Mar 23, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Apr 22, 2024
@fpvandoorn
Copy link
Member

This looks very nice from a readability standpoint. Also, the reactions on Zulip are overwhelmingly positive.
Am I correct that the current status of the delaboration is only that in is replaced by ?

@eric-wieser still had some reservations that he was planning to share (and I do worry a bit about the "can I rewrite the _ ∈ _?"), so please do. Otherwise I'll merge this in a few days.

@YaelDillies
Copy link
Collaborator

@eric-wieser
Copy link
Member

@eric-wieser still had some reservations that he was planning to share (and I do worry a bit about the "can I rewrite the _ ∈ _?"), so please do. Otherwise I'll merge this in a few days.

I've shared these in the original zulip thread.

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Apr 30, 2024
@fpvandoorn
Copy link
Member

Ah, thanks for the link.
I think we should merge this. The notation is a lot nicer, and it shouldn't be too bad for users to adapt to this.

bors d+
bors d=YaelDillies

@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Apr 30, 2024

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

@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Apr 30, 2024

✌️ YaelDillies 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 and removed awaiting-review The author would like community review of the PR labels Apr 30, 2024
@YaelDillies
Copy link
Collaborator

bors merge

@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Apr 30, 2024

👎 Rejected by label

@YaelDillies YaelDillies removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Apr 30, 2024
@YaelDillies
Copy link
Collaborator

bors merge

mathlib-bors bot pushed a commit that referenced this pull request Apr 30, 2024
Adds new syntax for sum/product big operators for `∑ x ∈ s, f x`. The set `s` can either be a `Finset` or a `Set` with a `Fintype` instance, in which case it is equivalent to `∑ x ∈ s.toFinset, f x`.

Also supports `∑ (x ∈ s) (y ∈ t), f x y` for `Finset.sum (s ×ˢ t) (fun ⟨x, y⟩ ↦ f x y)`. Note that these are not dependent products at the moment.

Adds `with` clauses, so for example `∑ (x ∈ Finset.range 5) (y ∈ Finset.range 5) with x < y, x * y`, which inserts a `Finset.filter` over the domain set.

Supports pattern matching in the variable position. This is by creating an experimental version of `extBinder` that uses `term:max` instead of `binderIdent`.

The new `∑ x ∈ s, f x` notation is used in `Algebra.BigOperators.Basic` for illustration, but the old `∑ x in s, f x` still works for backwards compatibility.


Zulip threads [here](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/.E2.88.91.20x.20.E2.88.88.20s.2C.20f.20x.20instead.20of.20.E2.88.91.20x.20in.20s.2C.20f.20x/near/387352818) and [here](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/Destruction.20and.20big.20operators/near/387366894)



Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Apr 30, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat: ∑ x ∈ s, f x to replace ∑ x in s, f x in the future [Merged by Bors] - feat: ∑ x ∈ s, f x to replace ∑ x in s, f x in the future Apr 30, 2024
@mathlib-bors mathlib-bors bot closed this Apr 30, 2024
@mathlib-bors mathlib-bors bot deleted the kmill_bigoperator_elem branch April 30, 2024 15:37
apnelson1 pushed a commit that referenced this pull request May 12, 2024
Adds new syntax for sum/product big operators for `∑ x ∈ s, f x`. The set `s` can either be a `Finset` or a `Set` with a `Fintype` instance, in which case it is equivalent to `∑ x ∈ s.toFinset, f x`.

Also supports `∑ (x ∈ s) (y ∈ t), f x y` for `Finset.sum (s ×ˢ t) (fun ⟨x, y⟩ ↦ f x y)`. Note that these are not dependent products at the moment.

Adds `with` clauses, so for example `∑ (x ∈ Finset.range 5) (y ∈ Finset.range 5) with x < y, x * y`, which inserts a `Finset.filter` over the domain set.

Supports pattern matching in the variable position. This is by creating an experimental version of `extBinder` that uses `term:max` instead of `binderIdent`.

The new `∑ x ∈ s, f x` notation is used in `Algebra.BigOperators.Basic` for illustration, but the old `∑ x in s, f x` still works for backwards compatibility.


Zulip threads [here](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/.E2.88.91.20x.20.E2.88.88.20s.2C.20f.20x.20instead.20of.20.E2.88.91.20x.20in.20s.2C.20f.20x/near/387352818) and [here](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/Destruction.20and.20big.20operators/near/387366894)



Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
callesonne pushed a commit that referenced this pull request May 16, 2024
Adds new syntax for sum/product big operators for `∑ x ∈ s, f x`. The set `s` can either be a `Finset` or a `Set` with a `Fintype` instance, in which case it is equivalent to `∑ x ∈ s.toFinset, f x`.

Also supports `∑ (x ∈ s) (y ∈ t), f x y` for `Finset.sum (s ×ˢ t) (fun ⟨x, y⟩ ↦ f x y)`. Note that these are not dependent products at the moment.

Adds `with` clauses, so for example `∑ (x ∈ Finset.range 5) (y ∈ Finset.range 5) with x < y, x * y`, which inserts a `Finset.filter` over the domain set.

Supports pattern matching in the variable position. This is by creating an experimental version of `extBinder` that uses `term:max` instead of `binderIdent`.

The new `∑ x ∈ s, f x` notation is used in `Algebra.BigOperators.Basic` for illustration, but the old `∑ x in s, f x` still works for backwards compatibility.


Zulip threads [here](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/.E2.88.91.20x.20.E2.88.88.20s.2C.20f.20x.20instead.20of.20.E2.88.91.20x.20in.20s.2C.20f.20x/near/387352818) and [here](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/Destruction.20and.20big.20operators/near/387366894)



Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated maintainer-merge t-meta Tactics, attributes or user commands
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants