-
Notifications
You must be signed in to change notification settings - Fork 297
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
refactor(topology/*): define and use dense_inducing #1193
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor nitpicks. Looks really good!
(assume a b, | ||
by rw [← coe_mul, extension_coe hf, extension_coe hf, extension_coe hf, is_ring_hom.map_mul f]) } | ||
|
||
instance top_ring_compl : topological_ring (completion α) := |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
exact hg c | ||
end | ||
|
||
lemma dense_range.inhabited (df : dense_range f) (b : β) : inhabited α := |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why inhabited
instead of nonempty
? since there is no canonical element, I think nonempty
is more natural -- and if you really need inhabited
for some applications, maybe it means that the statements you rely on should also use nonempty
instead of inhabited
, or otherwise you can easily derive inhabited
from nonempty
in the application.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inhabited
is needed because this is what lim
wants.
Looks very good to me (except for minor nitpicking). |
Traditionally, topology extends functions defined on dense subspaces (equipped by the induced topology). In mathlib, this was made type-theory-friendly by rather factoring through `dense_embedding` maps. A map `f : α → β` between topological spaces is a dense embedding if its image is dense, it is injective, and it pulls back the topology from `β` to the topology on `α`. It turns out that the injectivity was never used in any serious way. It used not to be used at all until we noticed it could be used to ensure the factorization equation `dense_embedding.extend_e_eq` could be made to hold without any assumption on the map to be extended. But of course this formalization trick is mathematically completely irrelevant. On the other hand, assuming injectivity prevents direct use in uniform spaces completion, because the map from a space to its (separated) completion is injective only when the original space is separated. This is why mathlib ring completion currently assumes a separated topological ring, and the perfectoid spaces project needed a lot of effort to drop that assumption. This commit makes all this completely painless. Along the way, we improve consistency and readability by turning a couple of conjunctions into structures. It also introduces long overdue fix to `function.uncurry` (which suffered from abusive pattern matching, similar to `prod.map`).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much @sgouezel and @jcommelin!
exact hg c | ||
end | ||
|
||
lemma dense_range.inhabited (df : dense_range f) (b : β) : inhabited α := |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inhabited
is needed because this is what lim
wants.
Co-Authored-By: Johan Commelin <johan@commelin.net> Co-Authored-By: sgouezel <sebastien.gouezel@univ-rennes1.fr>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I've fixed everything, and this is good to go.
@PatrickMassot The commit title also seems a bit funny. Did you mean to use the word "use"? |
For me, this is good to go. @jcommelin, once you're happy you can mark it as ready to merge. |
Indeed the PR title misses the word "use". I'm not sure I can safely change that (I don't think rewriting history would be good for this discussion). |
@sgouezel Great! It's on the merge queue. |
You can just click "edit" next to the PR title itself.
…On Tue, Jul 9, 2019, 3:09 AM Patrick Massot ***@***.***> wrote:
Indeed the PR title misses the word "use". I'm not sure I can safely
change that (I don't think rewriting history would be good for this
discussion).
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1193?email_source=notifications&email_token=AADUWBEXHCMDNDUKALD3TG3P6SEZNA5CNFSM4H634RCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZQGVTA#issuecomment-509635276>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AADUWBD4H2KFQBDVXWMRNNDP6SEZNANCNFSM4H634RCA>
.
|
…nity#1193) * refactor(topology/*): define and dense_inducing Traditionally, topology extends functions defined on dense subspaces (equipped by the induced topology). In mathlib, this was made type-theory-friendly by rather factoring through `dense_embedding` maps. A map `f : α → β` between topological spaces is a dense embedding if its image is dense, it is injective, and it pulls back the topology from `β` to the topology on `α`. It turns out that the injectivity was never used in any serious way. It used not to be used at all until we noticed it could be used to ensure the factorization equation `dense_embedding.extend_e_eq` could be made to hold without any assumption on the map to be extended. But of course this formalization trick is mathematically completely irrelevant. On the other hand, assuming injectivity prevents direct use in uniform spaces completion, because the map from a space to its (separated) completion is injective only when the original space is separated. This is why mathlib ring completion currently assumes a separated topological ring, and the perfectoid spaces project needed a lot of effort to drop that assumption. This commit makes all this completely painless. Along the way, we improve consistency and readability by turning a couple of conjunctions into structures. It also introduces long overdue fix to `function.uncurry` (which suffered from abusive pattern matching, similar to `prod.map`). * Apply suggestions from code review Co-Authored-By: Johan Commelin <johan@commelin.net> Co-Authored-By: sgouezel <sebastien.gouezel@univ-rennes1.fr> * minor fixes following review * Some more dot notation, consistent naming and field naming
Traditionally, topology extends functions defined on dense subspaces (equipped by the induced topology). In mathlib, this was made type-theory-friendly by rather factoring through
dense_embedding
maps. A mapf : α → β
between topological spaces is a dense embedding if its image is dense, it is injective, and it pulls back the topology fromβ
to the topology onα
. It turns outthat the injectivity was never used in any serious way. It used not to be used at all until we noticed it could be used to ensure the factorization equation
dense_embedding.extend_e_eq
could be made tohold without any assumption on the map to be extended. But of course this formalization trick is mathematically completely irrelevant.
On the other hand, assuming injectivity prevents direct use in uniform spaces completion, because the map from a space to its (separated) completion is injective only when the original space is separated. This is why mathlib ring completion currently assumes a separated topological ring, and the perfectoid spaces project needed a lot of effort to drop that assumption. This commit makes all this completely painless.
Along the way, we improve consistency and readability by turning a couple of conjunctions into structures. Also we introduce a predicate
dense_range
which was already everywhere, but without a name, and with duplicated proofs.It also introduces a long overdue fix to
function.uncurry
(which suffered from abusive patternmatching, similar to
prod.map
). Also removes the duplicationnhds_induced_eq_comap
/nhds_induced
.@sgouezel could you check I didn't harm your metric space completion and Gromov-Hausdorff work? It should be only little simplification, or a bit of help to the elaborator (especially a couple of
: _
added with speed).@rwbarton same question for the Stone-Cech compactification. The main difference should be that the map from a discrete space to ultrafilter on it is first proved to be dense inducing before adding that injectivity also holds. And then injectivity is no longer used anywhere. It means the
t2_space
assumption needs to be added a bit earlier but this should be totally irrelevant.See discussion at https://leanprover.zulipchat.com/#narrow/stream/116395-maths/topic/function.20extension and https://leanprover.zulipchat.com/#narrow/stream/116395-maths/topic/induced.20neighborhoods