-
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
[Merged by Bors] - feat(topology/covering): Define covering spaces #16087
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.
Thanks!
Aside from a trivial comment about code style, my two thoughts are:
- Since branched coverings are important, I'd be tempted to include a subset of the base as part of the data and do everything relative to that. In other words, we could define
is_covering_map_on
instead (and presumably as a convenience haveis_covering_map
, specializing this toset.univ
) and hopefully thus avoid some subtype pain in future work. - I'd be tempted to include the monodromy group as part of the data. I don't see a way to use the definitions here to make the statement that "the map
f : E → X
admits a set of trivializations with monodromy groupG
". In other words, it looks to me like we'll have to refactor or duplicate code when this day comes.
What do you think?
Feel free to push back, I think we could go with this for now at least.
Thanks! I hadn't though about branched covers. I've added I'm not so sure about including the monodromy group as part of the data. It seems like it would make the definition more clunky (e.g., path connectedness) and would make it more painful to prove that a given map is a covering map. Wouldn't it make more sense to just have the monodromy data be constructed from |
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.
Thanks!
On reflection I think it's probably best not to make the monodromy group explicit for now, especially as we do not have a theory of principal bundles.
One style suggestion, and one optional task but otherwise this LGTM. Thanks again.
bors d+
lemma continuous (hf : is_covering_map f) : continuous f := | ||
continuous_iff_continuous_at.mpr (λ x, (hf (f x)).continuous_at) | ||
|
||
lemma is_locally_homeomorph (hf : is_covering_map f) : is_locally_homeomorph f := |
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.
Optionally: I think it would be nice to generalise this to is_covering_map_on
(in the case when s
is open).
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'll have to think about this. It's a tad bit tricker than it looks, since you don't know that f ⁻¹' s
is open.
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.
you don't know that
f ⁻¹' s
is open.
I don't understand: f
is continuous and s
is open so this is trivial, right?
Thinking about this a bit more, I suppose you'll also need that s
is non-empty but otherwise this should be easy.
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.
Oh, are you talking about is_locally_homeomorph
? I was talking about continuous
.
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.
Is this along the lines of what you were thinking of? #17114
✌️ tb65536 can now approve this pull request. To approve and merge a pull request, simply reply with |
bors r+ |
This PR adds a definition of covering spaces. We don't want to have to specify a constant index set `I`, and we don't want to say "there exists an index set `I : Type u` such that `f ⁻¹' U` is homeomorphic to `I × U`" (annoying universe issues). Instead, we can use the preimage of any point as our index set. Supersedes #15276, where @alreadydone suggested that I drop the surjectivity assumption.
Pull request successfully merged into master. Build succeeded: |
This PR adds a definition of covering spaces.
We don't want to have to specify a constant index set
I
, and we don't want to say "there exists an index setI : Type u
such thatf ⁻¹' U
is homeomorphic toI × U
" (annoying universe issues). Instead, we can use the preimage of any point as our index set.Supersedes #15276, where @alreadydone suggested that I drop the surjectivity assumption.
embedding.discrete_topology
#16092