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: LocallyConstant.piecewise' #6589

Closed
wants to merge 14 commits into from

Conversation

dagurtomas
Copy link
Collaborator

@dagurtomas dagurtomas commented Aug 15, 2023

We define a variant of LocallyConstant.piecewise which defines a locally constant map on a subset given locally constant maps on two closed subsets covering it, that agree on the intersection.

Co-authored-by: Anatole Dedecker @ADedecker


Open in Gitpod

@dagurtomas dagurtomas added awaiting-review The author would like community review of the PR awaiting-CI labels Aug 15, 2023
@ADedecker ADedecker self-assigned this Aug 16, 2023
@ADedecker
Copy link
Member

ADedecker commented Aug 16, 2023

What do you think of the following implementation?

noncomputable def piecewise' {C₀ C₁ C₂ : Set X} (h₀ : C₀ ⊆ C₁ ∪ C₂) (h₁ : IsClosed C₁)
    (h₂ : IsClosed C₂) (f₁ : LocallyConstant C₁ Z) (f₂ : LocallyConstant C₂ Z)
    [DecidablePred (· ∈ C₁)] (hf : ∀ x (hx : x ∈ C₁ ∩ C₂), f₁ ⟨x, hx.1⟩ = f₂ ⟨x, hx.2⟩) :
    LocallyConstant C₀ Z :=
  letI : ∀ j : C₀, Decidable (j ∈ Subtype.val ⁻¹' C₁) := fun j ↦ decidable_of_iff (↑j ∈ C₁) Iff.rfl
  piecewise (h₁.preimage continuous_subtype_val) (h₂.preimage continuous_subtype_val)
    (by simpa [eq_univ_iff_forall] using h₀)
    (f₁.comap (restrictPreimage C₁ ((↑) : C₀ → X)))
    (f₂.comap (restrictPreimage C₂ ((↑) : C₀ → X))) <| by
      rintro ⟨x, hx₀⟩ ⟨hx₁ : x ∈ C₁, hx₂ : x ∈ C₂⟩
      simp_rw [coe_comap_apply _ _ continuous_subtype_val.restrictPreimage]
      exact hf x ⟨hx₁, hx₂⟩

It requires changing f.toFun to f in the definition of piecewise (we always use the coercion instead of .toFun, so we should stick to it), as well as adding the following after Continuous.codRestrict:

theorem Continuous.restrict {f : α → β} {s : Set α} {t : Set β} (h1 : MapsTo f s t)
    (h2 : Continuous f) : Continuous (h1.restrict f s t) :=
  (h2.comp continuous_subtype_val).codRestrict _

theorem Continuous.restrictPreimage {f : α → β} {s : Set β} (h : Continuous f) :
    Continuous (s.restrictPreimage f) :=
  h.restrict _

I can see a few disadvantages:

  • it's noncomputable (for bad reasons: Lean thinks it has to decide if some map is continuous in LocallyConstant.comap, but the map we use is always continuous)
  • unfolding this definition will be a bit harder. A way to make sure that this doesn't cause any problem is to add enough lemmas so that unfolding is no longer necessary.

@ADedecker ADedecker 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 Aug 16, 2023
@dagurtomas
Copy link
Collaborator Author

I like it! I tried it on my Nöbeling branch and it works well by adding a simple lemma about applying piecewise'. I'll implement the changes and push them here.

@ADedecker
Copy link
Member

Nice! By the way I opened #6616 with the two continuity lemmas on restrictions.

@ADedecker
Copy link
Member

Also could you add a comment saying that the two piecewise constructions should be generalized to ContinuousMap? We don't have a nice way to make the translation (I think?) so it's not really useful for now, but just adding a TODO would be nice.

@dagurtomas
Copy link
Collaborator Author

Regarding the noncomputability: LocallyConstant.comap is only noncomputable because you can comap along noncontinuous maps for some reason, right? Does anyone want to do this? Can we maybe have both versions, where one is computable and only accepts continuous maps?

@dagurtomas
Copy link
Collaborator Author

Regarding the noncomputability: LocallyConstant.comap is only noncomputable because you can comap along noncontinuous maps for some reason, right? Does anyone want to do this? Can we maybe have both versions, where one is computable and only accepts continuous maps?

There is a comment about this next to the definition of comap

@ADedecker
Copy link
Member

ADedecker commented Aug 16, 2023

Finally let me just mention that I think these two constructions could be made way nicer if we developed some gluing API. I'll try to find some time to post my thoughts in greater detail about this on Zulip, but what I imagine would be a proposition expressing that a given topological space is the gluing of a family of topological space (as a property on, not as a construction) that would allow to do this kind of constructions more systematically.

@ADedecker
Copy link
Member

Regarding the noncomputability: LocallyConstant.comap is only noncomputable because you can comap along noncontinuous maps for some reason, right? Does anyone want to do this? Can we maybe have both versions, where one is computable and only accepts continuous maps?

There is a comment about this next to the definition of comap

Indeed I think that would be a good idea, but maybe it's better to keep that aside for anther PR? Especially since I think we definitely want Johan's opinion on this kind of chance to see if it would mess with LTE.

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the blocked-by-other-PR This PR depends on another PR which is still in the queue. label Aug 16, 2023
@dagurtomas dagurtomas 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 Aug 16, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the blocked-by-other-PR This PR depends on another PR which is still in the queue. label Aug 19, 2023
@leanprover-community-mathlib4-bot
Copy link
Collaborator

This PR/issue depends on:

@ADedecker ADedecker removed the awaiting-review The author would like community review of the PR label Aug 21, 2023
Copy link
Member

@ADedecker ADedecker left a comment

Choose a reason for hiding this comment

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

Sorry for the delay. I made a few comments, but I think I shouldn't be the one merging that since I've made rather big suggestions (besides, I won't have time to take care of it in the near future). I'll un-assign myself, and you can ask for another review on Zulip after taking my review into account.

Mathlib/Topology/LocallyConstant/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Topology/LocallyConstant/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Topology/LocallyConstant/Basic.lean Outdated Show resolved Hide resolved
@ADedecker ADedecker 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 Sep 4, 2023
@bors bors bot changed the base branch from master to ScottCarnahan/BinomialRing2 September 17, 2023 03:26
@semorrison semorrison changed the base branch from ScottCarnahan/BinomialRing2 to master September 17, 2023 12:12
@dagurtomas dagurtomas added awaiting-review The author would like community review of the PR awaiting-CI and removed awaiting-author A reviewer has asked the author a question or requested changes labels Sep 27, 2023
@riccardobrasca riccardobrasca self-assigned this Sep 27, 2023
Copy link
Member

@riccardobrasca riccardobrasca left a comment

Choose a reason for hiding this comment

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

Thanks!

bors d+

Mathlib/Topology/LocallyConstant/Basic.lean Show resolved Hide resolved
Mathlib/Topology/LocallyConstant/Basic.lean Show resolved Hide resolved
@bors
Copy link

bors bot commented Sep 28, 2023

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

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added delegated and removed awaiting-review The author would like community review of the PR labels Sep 28, 2023
@dagurtomas
Copy link
Collaborator Author

bors r+

bors bot pushed a commit that referenced this pull request Sep 28, 2023
We define a variant of `LocallyConstant.piecewise` which defines a locally constant map on a subset given locally constant maps on two closed subsets covering it, that agree on the intersection.

-  [x] depends on: #6616 

Co-authored-by: Anatole Dedecker @ADedecker
@bors
Copy link

bors bot commented Sep 28, 2023

Pull request successfully merged into master.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title feat: LocallyConstant.piecewise' [Merged by Bors] - feat: LocallyConstant.piecewise' Sep 28, 2023
@bors bors bot closed this Sep 28, 2023
@bors bors bot deleted the dagur_piecewise branch September 28, 2023 08:56
bors bot pushed a commit that referenced this pull request Nov 1, 2023
Nobeling's theorem: the Z-module of locally constant maps from a profinite set to the integers is free.

-  [x] depends on: #6360 
-  [x] depends on: #6373 
-  [x] depends on: #6395 
-  [x] depends on: #6396  
-  [x] depends on: #6432
-  [x] depends on: #6520
-  [x] depends on: #6578
-  [x] depends on: #6589 
-  [x] depends on: #6722 
-  [x] depends on: #7829 
-  [x] depends on: #7895 
-  [x] depends on: #7896 
-  [x] depends on: #7897  
-  [x] depends on: #7899
fgdorais pushed a commit that referenced this pull request Nov 1, 2023
Nobeling's theorem: the Z-module of locally constant maps from a profinite set to the integers is free.

-  [x] depends on: #6360 
-  [x] depends on: #6373 
-  [x] depends on: #6395 
-  [x] depends on: #6396  
-  [x] depends on: #6432
-  [x] depends on: #6520
-  [x] depends on: #6578
-  [x] depends on: #6589 
-  [x] depends on: #6722 
-  [x] depends on: #7829 
-  [x] depends on: #7895 
-  [x] depends on: #7896 
-  [x] depends on: #7897  
-  [x] depends on: #7899
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants