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] - refactor: use the typeclass SProd to implement overloaded notation · ×ˢ · #4200

Closed
wants to merge 28 commits into from

Conversation

Komyyy
Copy link
Collaborator

@Komyyy Komyyy commented May 22, 2023

Currently, the following notations are changed from · ×ˢ · because Lean 4 can't deal with ambiguous notations.

Definition Notation
Finset.product · ×ᶠ ·
LowerSet.prod · ×ˡˢ ·
UpperSet.prod · ×ᵘˢ ·

And following notations were still · ×ˢ · but they causes performance issues; even timeouts.

Definition Notation
Set.prod · ×ˢ ·
List.product · ×ˢ ·
Multiset.product · ×ˢ ·

This PR makes the new typeclass SProd to implement overloaded notation · ×ˢ ·, and revert changed notations.
Also, this PR changes the notation of Filter.prod from · ×ᶠ · to · ×ˢ ·.


Open in Gitpod

@semorrison semorrison added the merge-conflict The PR has a merge conflict with master, and needs manual merging. label May 24, 2023
@semorrison semorrison removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. label May 24, 2023
Copy link
Member

@ChrisHughes24 ChrisHughes24 left a comment

Choose a reason for hiding this comment

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

I'm very happy with these changes, but they're quite sweeping so I'll let somebody else have an opinion as well.

@Komyyy Komyyy added WIP Work in progress and removed awaiting-review labels May 24, 2023
@kmill
Copy link
Contributor

kmill commented May 24, 2023

I'm happy with these changes too. I know I had some reservations on Zulip, but I reviewed all the changes in closer detail and they're all improvements, so I say we go ahead.

Want to put it back at awaiting-review @Komyyy ? Like Chris the changes are quite sweeping and I'd like someone else to look at it (I'm fine with Chris deciding to merge it).

@semorrison semorrison added the merge-conflict The PR has a merge conflict with master, and needs manual merging. label May 26, 2023
@semorrison semorrison removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. label May 27, 2023
@semorrison
Copy link
Contributor

bors d+

@bors
Copy link

bors bot commented May 28, 2023

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

@Komyyy
Copy link
Collaborator Author

Komyyy commented May 28, 2023

bors r+

@urkud
Copy link
Member

urkud commented May 28, 2023

Historical remark: I introduced a similar typeclass in leanprover-community/mathlib#11300, then @YaelDillies replaced it with overlapping notations in leanprover-community/mathlib#15717 because it worked better in Lean 3.

@bors
Copy link

bors bot commented May 28, 2023

Merge conflict.

@ChrisHughes24
Copy link
Member

bors r+

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label May 28, 2023
@kmill
Copy link
Contributor

kmill commented May 28, 2023

@urkud To add to the historical remark: one thing that is different now from then is that instances of this SProd class make use of a custom fbinop% elaborator that give us similar benefits to the Lean 3 overlapping notations despite being a class-based design. (I pinged you and Yaël earlier in this thread to show a line that couldn't work with a Lean 3 class, but could work with Lean 3 overlapping notation, and now still works with Lean 4 classes.)

bors bot pushed a commit that referenced this pull request May 28, 2023
…`· ×ˢ ·` (#4200)

Currently, the following notations are changed from `· ×ˢ ·` because Lean 4 can't deal with ambiguous notations.
| Definition | Notation |
| :

Co-authored-by: Jeremy Tan Jie Rui <reddeloostw@gmail.com>
Co-authored-by: Kyle Miller <kmill31415@gmail.com>
Co-authored-by: Chris Hughes <chrishughes24@gmail.com>
@bors
Copy link

bors bot commented May 28, 2023

Pull request successfully merged into master.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title refactor: use the typeclass SProd to implement overloaded notation · ×ˢ · [Merged by Bors] - refactor: use the typeclass SProd to implement overloaded notation · ×ˢ · May 28, 2023
@bors bors bot closed this May 28, 2023
@bors bors bot deleted the Komyyy/Set.prod branch May 28, 2023 14:10
qawbecrdtey pushed a commit to qawbecrdtey/greedoid-mathlib4 that referenced this pull request Jun 12, 2023
…`· ×ˢ ·` (leanprover-community#4200)

Currently, the following notations are changed from `· ×ˢ ·` because Lean 4 can't deal with ambiguous notations.
| Definition | Notation |
| :

Co-authored-by: Jeremy Tan Jie Rui <reddeloostw@gmail.com>
Co-authored-by: Kyle Miller <kmill31415@gmail.com>
Co-authored-by: Chris Hughes <chrishughes24@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated ready-to-merge This PR has been sent to bors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants