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: The diagonal of a finset #13899

Closed
wants to merge 3 commits into from
Closed

Conversation

YaelDillies
Copy link
Collaborator

From LeanAPAP


Open in Gitpod

Copy link

github-actions bot commented Jun 17, 2024

PR summary bd289da70a

Import changes for modified files

Dependency changes

File Base Count Head Count Change
Mathlib.Data.Finset.Pi 411 412 +1 (+0.24%)
Mathlib.Data.Fintype.Pi 418 419 +1 (+0.24%)
Import changes for all files
Files Import difference
4 files Mathlib.Data.Fin.Tuple.Finset Mathlib.Data.Finset.Pi Mathlib.Data.Fintype.Pi Mathlib.Data.Fintype.Vector
1

Declarations diff

+ card_piDiag
+ mem_piDiag
+ piDiag
+ piDiag_subset_piFinset
+ piFinset_filter_const

You can run this locally as follows
## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/declarations_diff.sh long <optional_commit>

variable {ι : Type*} [DecidableEq (ι → α)] {s : Finset α} {f : ι → α}

/-- The diagonal of a finset `s : Finset α` as a finset of functions `ι → α`. -/
def piDiag (s : Finset α) (ι : Type*) [DecidableEq (ι → α)] : Finset (ι → α) := s.image (const ι)
Copy link
Member

Choose a reason for hiding this comment

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

Does .map work here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Subtly, no: If \iota is empty, then const \iota is not injective. Of course in that case the finset is obvious to construct, but I can't case on whether \iota is empty or not (unless I assume Decidable (IsEmpty \iota) which is.. unorthodox)

Copy link
Member

Choose a reason for hiding this comment

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

Does the empty case make any sense in your application?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It would be kind of strange as my application is \iota := Fin n and I would have to rule out n = 0 explicitly. Not the end of the world, though.


variable {ι : Type*} [DecidableEq (ι → α)] {s : Finset α} {f : ι → α}

/-- The diagonal of a finset `s : Finset α` as a finset of functions `ι → α`. -/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe this is just my ignorance, but I'm not sure what a diagonal is after reading this comment

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Better now?

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps foreshadow what this is for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure my use case is a good fit for the docstring. It is used inside a proof of Almost Periodicity, where we are interested in tuples x and y differing by a constant, and we are interested in tuples because we can use a concentration inequality to approximate some random variable X by an average of uniform variables x 0, ..., x (n - 1) valued in some set.

You can see it for yourself here: https://github.com/search?q=repo%3AYaelDillies%2FLeanAPAP%20wideDiag&type=code (it's currently called wideDiag in APAP)


/-- The diagonal of a finset `s : Finset α` as a finset of functions `ι → α`, namely the set of
constant functions valued in `s`. -/
def piDiag (s : Finset α) (ι : Type*) [DecidableEq (ι → α)] : Finset (ι → α) := s.image (const ι)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would constFnsWithin be better? I think a niche notion like this can afford to have a long name, and should have a name that is clear.

Copy link
Collaborator Author

@YaelDillies YaelDillies Jul 18, 2024

Choose a reason for hiding this comment

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

I see your point, but it is completely analogous to Finset.diag, except for ι → α rather than α × α. I think this justifies the current name (and points to the fact that we might want the ι →₀ α version in the future, which is not something your naming scheme would allow)

Copy link
Collaborator

@Ruben-VandeVelde Ruben-VandeVelde left a comment

Choose a reason for hiding this comment

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

Fine for me.

maintainer merge

Copy link

🚀 Pull request has been placed on the maintainer queue by Ruben-VandeVelde.

@riccardobrasca
Copy link
Member

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit bd289da.
There were no significant changes against commit 2d49bad.

@riccardobrasca
Copy link
Member

Thanks!

bors merge

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Jul 20, 2024
mathlib-bors bot pushed a commit that referenced this pull request Jul 20, 2024
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Jul 20, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat: The diagonal of a finset [Merged by Bors] - feat: The diagonal of a finset Jul 20, 2024
@mathlib-bors mathlib-bors bot closed this Jul 20, 2024
@mathlib-bors mathlib-bors bot deleted the pi_diag branch July 20, 2024 11:58
judithludwig pushed a commit that referenced this pull request Jul 20, 2024
@adomani adomani mentioned this pull request Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer-merge ready-to-merge This PR has been sent to bors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants