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(topology/connected): Connectedness of unions of sets #10005

Closed
wants to merge 39 commits into from

Conversation

AntoineChambert-Loir
Copy link
Collaborator

@AntoineChambert-Loir AntoineChambert-Loir commented Oct 27, 2021

  • Add multiple results about when unions of sets are (pre)connected. In particular, the union of connected sets indexed by such that each set intersects the next is connected.
  • Remove some set. prefixes in the file
  • There are two minor fixes in other files, presumably caused by the fact that they now import order.succ_pred
  • Co-authored by Floris van Doorn fpvdoorn@gmail.com

Open in Gitpod

Add a theorem that unions of preconnected sets indexed by integers
such that each of them meets the next one is preconnected

In a separate file (src/data/set/increasing_union.lean),
add a series of possibly lemma on increasing unions
It is possible that these lemmas have a neater proof.
@AntoineChambert-Loir AntoineChambert-Loir added the awaiting-review The author would like community review of the PR label Oct 27, 2021
src/data/set/increasing_union.lean Outdated Show resolved Hide resolved
src/data/set/increasing_union.lean Outdated Show resolved Hide resolved

Given a sequence of sets s : ℕ → set α,
we construct its increasing Union by
λ n:ℕ, Union (set restrict s { j : ℕ | j ≤ n}) -/
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure you don't want

Suggested change
λ n:ℕ, Union (set restrict s { j : ℕ | j ≤ n}) -/
λ n:ℕ, Union (subtype.restrict s (≤ n)) -/

Copy link
Member

Choose a reason for hiding this comment

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

This also looks a lot like partial_sups, cc @YaelDillies

Copy link
Member

Choose a reason for hiding this comment

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

And accumulate.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah it is partial_sups. I'm sorry you wrote it all, Antoine, because this is duplicate 😕

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 was almost certain something like that would exist, and I am truly glad that it already exists.
I'll try to understand how it works tomorrow and to adjust the other file accordingly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@YaelDillies , OK, it was nice and easy to use partial_sups.
At first, I thought that the assertion that partial_sups s is monotone was missing, but it seems to be included in the galois connection…

Copy link
Collaborator

Choose a reason for hiding this comment

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

partial_sups is defined as a monotone function. That's what the little m to the right of the arrow stands for. You can look at the API for preorder_hom for how to use this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately, I can't read that little symbol on the right of the arrow, it is printed as a box in both vim and VisualStudio, all I know is that it has Unicode code 8344…

Copy link
Member

Choose a reason for hiding this comment

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

@AntoineChambert-Loir In that case I think you are in dire need of a font with more unicode support 😉

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That I could guess, but I think I use the standard VisualStudioCode (on Mac OS)…

src/topology/connected.lean Outdated Show resolved Hide resolved
@AntoineChambert-Loir AntoineChambert-Loir added incomplete and removed awaiting-review The author would like community review of the PR labels Oct 28, 2021
@AntoineChambert-Loir
Copy link
Collaborator Author

I left unfortunate bugs in the commit (bad interaction between my two branches).
Moreover, the theorems on monotone Unions are essentially, I may simplify them.

A new commit will appear soon.

@AntoineChambert-Loir
Copy link
Collaborator Author

Finally, I left the two theorems is_preconnected.Union_of_monotone and is_preconnected.sUnion_of_filtered, but I wonder whether they should remain in this form.
Indeed, at least under the assumption that the sets are nonempty, they follow rightly from is_preconnected.Union. And in the case where I use them, the sets are nonempty.
What do you think?

@AntoineChambert-Loir AntoineChambert-Loir added awaiting-review The author would like community review of the PR and removed incomplete labels Oct 28, 2021
Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

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

Some initial style comments

src/topology/connected.lean Outdated Show resolved Hide resolved
src/topology/connected.lean Outdated Show resolved Hide resolved
src/topology/connected.lean Outdated Show resolved Hide resolved
src/topology/connected.lean Outdated Show resolved Hide resolved
src/topology/connected.lean Outdated Show resolved Hide resolved
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
src/topology/connected.lean Outdated Show resolved Hide resolved
@jcommelin jcommelin changed the title feat(src/topology/connected.lean): add a theorem on connected unions feat(src/topology/connected): add a theorem on connected unions Nov 4, 2021
@PatrickMassot PatrickMassot 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 Nov 5, 2021
@YaelDillies YaelDillies changed the title feat(src/topology/connected): add a theorem on connected unions feat(topology/connected): A increasing union indexed by a chain is connected Nov 5, 2021
@PatrickMassot
Copy link
Member

@AntoineChambert-Loir , I worked on the first two lemmas to show you some tricks and answer some questions. My advice is to study the diff at 1a1bb97 and see whether you can learn something and clean up this PR. Unfortunately git is a bit confused when displaying the diff (because it can't figure out which of my blank lines match yours) so you may have to create a new version of the file on your computer and compare them by putting them side by side.

@leanprover-community-bot-assistant leanprover-community-bot-assistant 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 Jan 17, 2022
@fpvandoorn
Copy link
Member

Thanks for the review! It is indeed much nicer to do this for is_succ_archimedean (I didn't know we had this type class).
To make the review process easier, I moved all the stuff I added about relations in a separate PR: #11518.

@YaelDillies
Copy link
Collaborator

Yeah, this is all fairly new. Nothing in mathlib uses it yet, so I'm very happy to see people needing it!

@leanprover-community-bot-assistant leanprover-community-bot-assistant 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 Jan 28, 2022
@leanprover-community-bot-assistant
Copy link
Collaborator

@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Jan 28, 2022
@leanprover-community-bot-assistant leanprover-community-bot-assistant removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Jan 28, 2022
@fpvandoorn fpvandoorn 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 Feb 1, 2022
@fpvandoorn fpvandoorn changed the title feat(topology/connected): A increasing union indexed by a chain is connected feat(topology/connected): Connectedness of unions Feb 2, 2022
@fpvandoorn fpvandoorn changed the title feat(topology/connected): Connectedness of unions feat(topology/connected): Connectedness of unions of sets Feb 2, 2022
Copy link
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

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

Thanks 🎉 It took a while, but this looks really great now!

bors merge

@leanprover-community-bot-assistant leanprover-community-bot-assistant added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Feb 2, 2022
bors bot pushed a commit that referenced this pull request Feb 2, 2022
* Add multiple results about when unions of sets are (pre)connected. In particular, the union of connected sets indexed by `ℕ` such that each set intersects the next is connected.
* Remove some `set.` prefixes in the file
* There are two minor fixes in other files, presumably caused by the fact that they now import `order.succ_pred`
* Co-authored by Floris van Doorn fpvdoorn@gmail.com



Co-authored-by: Antoine Chambert-Loir <antoine.chambert-loir@math.univ-paris-diderot.fr>
Co-authored-by: Patrick Massot <patrickmassot@free.fr>
Co-authored-by: Floris van Doorn <fpvdoorn@gmail.com>
theorem is_connected.Union_nat_of_chain {s : ℕ → set α}
(H : ∀ n : ℕ, is_connected (s n))
(K : ∀ n : ℕ, (s n ∩ s n.succ).nonempty) :
/-- The Union of connected sets indexed by a type with an archimedian successor (like `ℕ` or `ℤ`)
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
/-- The Union of connected sets indexed by a type with an archimedian successor (like `ℕ` or `ℤ`)
/-- The Union of connected sets indexed by a type with an archimedean successor (like `ℕ` or `ℤ`)


/-- The Union of connected sets indexed by a type with an archimedian successor (like `ℕ` or `ℤ`)
such that any two neighboring sets meet is preconnected. -/
theorem is_preconnected.Union_of_chain {s : β → set α}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would get rid of dot notation here and below because you can't actually use it.

Copy link
Member

Choose a reason for hiding this comment

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

I agree you cannot actually use it (though I kinda hope we can change the way projection notation works in Lean 4 so that H.Union_of_chain K works in this case).
However, I'm not sure if that means we should change the . to a _: it might be more predictable to guess the name if we consistently keep using . even when projection notation doesn't apply.

Copy link
Collaborator

Choose a reason for hiding this comment

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

At least, there's precedent for not using it when the condition is in a forall. For example, star_convex_Union.

Copy link
Member

@fpvandoorn fpvandoorn Feb 3, 2022

Choose a reason for hiding this comment

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

I believe there is a bunch of precedent for both options. We have not decided on a rule here. (measurable_set.pi to pick one)

@bors
Copy link

bors bot commented Feb 2, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(topology/connected): Connectedness of unions of sets [Merged by Bors] - feat(topology/connected): Connectedness of unions of sets Feb 2, 2022
@bors bors bot closed this Feb 2, 2022
@bors bors bot deleted the ad-connectedness branch February 2, 2022 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

None yet

8 participants