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(topology/subset_properties): more properties of compact_covering #6328

Closed
wants to merge 14 commits into from

Conversation

urkud
Copy link
Member

@urkud urkud commented Feb 20, 2021

Modify the definition of compact_covering α n to ensure that it is monotone in n.

Also, in a locally compact space, prove the existence of a compact exhaustion, i.e. a sequence which satisfies the properties for compact_covering and in which, moreover, the interior of the next set includes the previous set.


@github-actions github-actions 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 Feb 20, 2021
@github-actions github-actions 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 Feb 22, 2021
@urkud urkud added the awaiting-review The author would like community review of the PR label Feb 22, 2021
@github-actions
Copy link

🎉 Great news! Looks like all the dependencies have been resolved:

💡 To add or remove a dependency please update this issue/PR description.

Brought to you by Dependent Issues (:robot: ). Happy coding!

@hrmacbeth
Copy link
Member

hrmacbeth commented Feb 24, 2021

Given the redefinition to require that the covering be monotone, I propose renaming to compact_exhaustion, which is standard enough terminology that (eg) it is used on Wikipedia; what do you think?

Edit -- I read your definition more closely, I see that your definition does not always have the "closure subset interior" property, so my suggestion was wrong.

But how about adding the definition

def compact_exhaustion (K : ℕ → set α) : Prop := (∀ n, is_compact (K n)) ∧
  (∀ n, K n ⊆ interior (K (n + 1))) ∧ (⋃ n, K n) = univ

(probably as a structure), and separately changing the definition of compact_covering to require monotonicity (without the part about upgrading it if possible) -- then just making two separate series of lemmas? It's not immediately clear to me why it would be beneficial to combine the two definitions in an "if possible, extra properties" way, as you do.

@hrmacbeth
Copy link
Member

Also, is [sigma_compact_space α] equivalent to (cocompact α).is_countably_generated? Might this redefinition streamline some proofs? I could try it sometime if you think it's a good idea.

@urkud urkud 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 Feb 24, 2021
@urkud
Copy link
Member Author

urkud commented Feb 24, 2021

You're right, it's easier to have two definitions. Modified.

@urkud urkud 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 24, 2021
@urkud
Copy link
Member Author

urkud commented Feb 24, 2021

Also, is [sigma_compact_space α] equivalent to (cocompact α).is_countably_generated? Might this redefinition streamline some proofs? I could try it sometime if you think it's a good idea.

Seems to be true but I'm not sure that this is useful for golfing proofs.

@urkud urkud added this to In progress in Partition of unity via automation Feb 24, 2021
Copy link
Member

@hrmacbeth hrmacbeth left a comment

Choose a reason for hiding this comment

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

Nice! This review is mostly docstring suggestions, feel free to take or leave. (I made a rough edit of the PR comment, too.)

src/topology/subset_properties.lean Outdated Show resolved Hide resolved
src/topology/subset_properties.lean Outdated Show resolved Hide resolved
src/topology/subset_properties.lean Show resolved Hide resolved
src/topology/subset_properties.lean Outdated Show resolved Hide resolved
src/topology/subset_properties.lean Outdated Show resolved Hide resolved
Co-authored-by: hrmacbeth <25316162+hrmacbeth@users.noreply.github.com>
@hrmacbeth
Copy link
Member

I have a question about strategy. You could have made a definition

structure compact_exhaustion := 
(K : ℕ → set α) 
(compact : ∀ n, is_compact (K n))
(monotone : ∀ n, K n ⊆ interior (K (n + 1)))
(union : (⋃ n, K n) = univ)

(or some variant, eg an unbundled version

structure is_compact_exhaustion (K : ℕ → set α) : Prop := 
(compact : ∀ n, is_compact (K n))
(monotone : ∀ n, K n ⊆ interior (K (n + 1)))
(union : (⋃ n, K n) = univ)

) and then proved the lemmas such as compact_exhaustion_subset and
compact_exhaustion_subset_interior not just for the particular compact_exhaustion produced by choice, but for all compact exhaustions. (Possibly useful if, in the future, one constructs a special compact exhaustion for a proof, which has certain other properties.). Is there a reason you didn't do it this way?

@urkud
Copy link
Member Author

urkud commented Feb 24, 2021

The main reason is that we have only two lemmas not included in the definition, and they follow from is_compact_exhaustion.monotone in 1-2 lines of code.

UPD: it seems that some proofs i #6395 can be golfed using is_compact_exhaustion, lemmas, and operation "prepend empty set".

@urkud urkud 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 Feb 24, 2021
@urkud urkud 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 25, 2021
Copy link
Member

@hrmacbeth hrmacbeth left a comment

Choose a reason for hiding this comment

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

bors d+

Thank you for adding the compact_exhaustion structure!

If the linter is happy, feel free to merge.

topological space is a sequence of compact sets `K n` such that `K n ⊆ interior (K (n + 1))` and
`(⋃ n, K n) = univ`.

If `X` is a locally compact sigma compact space, then `compact_exhaustion.some X` provides a choice
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If `X` is a locally compact sigma compact space, then `compact_exhaustion.some X` provides a choice
If `X` is a locally compact sigma compact space, then `compact_exhaustion.choice X` provides a choice

(I don't have an opinion on whether it's better to use some or choice, but I think you use choice below.)

Comment on lines 807 to +808
def compact_covering : ℕ → set α :=
classical.some exists_compact_covering
accumulate exists_compact_covering.some
Copy link
Member

Choose a reason for hiding this comment

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

Do you need the monotonicity of the general compact covering, now that you have a separate construction of compact exhaustion in the locally compact case? You could revert the edits here and on the next two lemmas. But on the other hand, it does no harm and might be useful at some point, so if you want to leave it, that's also fine with me.

Copy link
Member Author

Choose a reason for hiding this comment

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

I would prefer to leave accumulate here because the new definition is at least as useful as the old one.

@bors
Copy link

bors bot commented Feb 25, 2021

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

@urkud
Copy link
Member Author

urkud commented Feb 25, 2021

bors merge

@github-actions github-actions bot 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 25, 2021
bors bot pushed a commit that referenced this pull request Feb 25, 2021
…ering` (#6328)

Modify the definition of `compact_covering α n` to ensure that it is monotone in `n`.

Also, in a locally compact space, prove the existence of a compact exhaustion, i.e. a sequence which satisfies the properties for `compact_covering` and in which, moreover, the interior of the next set includes the previous set.
@bors
Copy link

bors bot commented Feb 26, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title refactor(topology/subset_properties): more properties of compact_covering [Merged by Bors] - refactor(topology/subset_properties): more properties of compact_covering Feb 26, 2021
@bors bors bot closed this Feb 26, 2021
Partition of unity automation moved this from In progress to Done Feb 26, 2021
@bors bors bot deleted the compact-covering branch February 26, 2021 00:42
b-mehta pushed a commit that referenced this pull request Apr 2, 2021
…ering` (#6328)

Modify the definition of `compact_covering α n` to ensure that it is monotone in `n`.

Also, in a locally compact space, prove the existence of a compact exhaustion, i.e. a sequence which satisfies the properties for `compact_covering` and in which, moreover, the interior of the next set includes the previous set.
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
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants