Skip to content
This repository was archived by the owner on Jul 24, 2024. It is now read-only.

feat(group_theory/monoid_localization): some homs induced on localizations: lift, map#2118

Merged
mergify[bot] merged 41 commits intomasterfrom
mon_loc
Apr 3, 2020
Merged

feat(group_theory/monoid_localization): some homs induced on localizations: lift, map#2118
mergify[bot] merged 41 commits intomasterfrom
mon_loc

Conversation

@101damnations
Copy link
Collaborator

The second third of monoid_localization. The linter complained about loads of @[simp]s and I don't understand why (including some that made it through on the last PR). Maybe I need to experiment or read up on the linter, but I've removed the @[simp]s for now, anyway.

The additive versions of the away_map stuff will be in the next PR.

TO CONTRIBUTORS:

Make sure you have:

  • reviewed and applied the coding style: coding, naming
  • reviewed and applied the documentation requirements
  • for tactics:
  • make sure definitions and lemmas are put in the right files
  • make sure definitions and lemmas are not redundant

If this PR is related to a discussion on Zulip, please include a link in the discussion.

For reviewers: code review check list

@101damnations
Copy link
Collaborator Author

101damnations commented Mar 10, 2020

I have no idea why this is failing to build due to 'deep recursion' in algebra.category.Mon.basic and I'm not sure what to do about it either. I haven't touched that file. It compiles when I checkout to master though. I dunno what I've done.

Edit: Wow, doing what errors say really does work. Increased stack size and it seems to compile now

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.

On first reading, this looks really good. @ChrisHughes24 do you have some comments?

/-- If `f : M →* N` is a localization map for a submonoid `S` and `k : N ≃* P` is an isomorphism
of `comm_monoid`s, `k ∘ f` is a localization map for `M` at `S`. -/
@[to_additive "If `f : M →+ N` is a localization map for a submonoid `S` and `k : N ≃+ P` is an isomorphism of `add_comm_monoid`s, `k ∘ f` is a localization map for `M` at `S`."]
def mul_equiv_map (k : N ≃* P) :
Copy link
Member

Choose a reason for hiding this comment

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

I think the following definitions should come with some compatibility lemmas. E.g., the coe to function of this mul_equiv_map is k ∘ f.1. But also, what happens if I compose this with k.symm: then I recover f. Similar lemmas apply below.

Copy link
Member

Choose a reason for hiding this comment

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

@101damnations What do you think of ⬆️ remark?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added some in this commit and the latest commit but I might be missing some obvious ones, I'm not sure

@robertylewis robertylewis 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 Mar 28, 2020
@kim-em
Copy link
Collaborator

kim-em commented Mar 30, 2020

@ChrisHughes24, @101damnations, where are we up to on this one? I've lost track, and can't quite tell from the unresolved conversations above which issues are ongoing or not. Does it help to have extra eyes here, or is this almost ready to go?

@ChrisHughes24
Copy link
Member

The one comment I have now, is that a lot of the monoid localisation file is quite difficult to read. Some docstribgs would be good so I can understand the statements of the theorems.

@101damnations
Copy link
Collaborator Author

I've not added doc strings at the end for now; those lemmas are about functions with unwieldy names but are just kind of unfolding the definitions... Apart from maybe f.mul_equiv_of_mul_equiv k H x = f.map (a proof) k x, but that's true by rfl.

@jcommelin jcommelin requested a review from ChrisHughes24 April 2, 2020 07:05
Comment on lines 289 to 295
/-- Application of the 'weaker-than-injectivity' property of localization maps `f : M →* N`
for a submonoid `S ⊆ M` to the fact that for all `x₁ x₂ : M` and `y₁, y₂ ∈ S`,
`f x₁ * (f y₁)⁻¹ * f y₂ = f x₂ → f (x₁ * y₂) = f (x₂ * y₁)`. -/
@[to_additive "Application of the 'weaker-than-injectivity' property of localization maps `f : M →+ N` for a submonoid `S ⊆ M` to the fact that for all `x₁ x₂ : M` and `y₁, y₂ ∈ S`, `(f x₁ - f y₁) + f y₂ = f x₂ → f (x₁ + y₂) = f (x₂ + y₁)`."]
lemma exists_of_sec_mk' (x) (y : S) :
∃ c : S, x * (sec S f.1 $ f.mk' x y).2 * c = (sec S f.1 $ f.mk' x y).1 * y * c :=
(f.4 _ _).1 $ f.mk'_eq_iff_eq.1 $ (mk'_sec _ _).symm
Copy link
Member

Choose a reason for hiding this comment

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

This docstring doesn't seem to match the statement of the theorem.

let ⟨c, hc⟩ := (f.4 _ _).1 h in (k.4 _ _).2
⟨⟨g c, hg c⟩, show g _ * g _ = g _ * g _, by rw [←g.map_mul, hc, g.map_mul]⟩

@[to_additive] lemma eq_of_eq (hg : ∀ y : S, is_unit (g y)) {x y} (h : f.1 x = f.1 y) :
Copy link
Member

Choose a reason for hiding this comment

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

I think this needs a docstring.

@[simp, to_additive] lemma lift_id (x) : f.lift f.2 x = x :=
monoid_hom.ext_iff.1 (f.lift_of_comp $ monoid_hom.id N) x

@[simp, to_additive] lemma lift_inv {k : localization_map S P} (z : N) :
Copy link
Member

Choose a reason for hiding this comment

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

I think this needs a docstring

map for `S` and `k : P ≃* M` is an isomorphism of `comm_monoid`s such that `k(T) = S`, `f ∘ k`
is a localization map for `T`. -/
@[to_additive "Given `comm_monoid`s `M, P` and submonoids `S ⊆ M, T ⊆ P`, if `f : M →* N` is a localization map for `S` and `k : P ≃* M` is an isomorphism of `comm_monoid`s such that `k(T) = S`, `f ∘ k` is a localization map for `T`."]
def mul_equiv_comap {k : P ≃* M} (H : T.map k.to_monoid_hom = S) :
Copy link
Member

Choose a reason for hiding this comment

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

To me the name comap means there's a contravariant functor somewhere. There might be a better name.

/-- If `f : M →* N` is a localization map for a submonoid `S` and `k : N ≃* P` is an isomorphism
of `comm_monoid`s, `k ∘ f` is a localization map for `M` at `S`. -/
@[to_additive "If `f : M →+ N` is a localization map for a submonoid `S` and `k : N ≃+ P` is an isomorphism of `add_comm_monoid`s, `k ∘ f` is a localization map for `M` at `S`."]
def to_mul_equiv (k : N ≃* P) :
Copy link
Member

Choose a reason for hiding this comment

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

to_mul_equiv sounds like it should make a mul_equiv

Also, @jcommelin do you think this might cause problems when in practice there is some canonical map M->P floating around that is eq but not defeq to the map given by this definition.

Copy link
Member

Choose a reason for hiding this comment

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

That will probably be painful, yes... But maybe we should save that for a later refactor, because this is something that we don't have a good solution for anyways...

@ChrisHughes24
Copy link
Member

@jcommelin any further comments before I merge this?

@jcommelin
Copy link
Member

Nope, let's go!

@ChrisHughes24 ChrisHughes24 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 Apr 3, 2020
@mergify mergify bot merged commit a590d4b into master Apr 3, 2020
@mergify mergify bot deleted the mon_loc branch April 3, 2020 12:34
butterthebuddha pushed a commit to butterthebuddha/mathlib that referenced this pull request May 15, 2020
…tions: lift, map (leanprover-community#2118)

* should I be changing and committing toml idk

* initial monoid loc lemmas

* responding to PR comments

* removing bad @[simp]

* inhabited instances

* remove #lint

* additive inhabited instance

* using is_unit & is_add_unit

* doc string

* remove simp

* submonoid.monoid_loc... -> submonoid.localization

* submonoid.monoid_loc... -> submonoid.localization

* generalize inhabited instance

* remove inhabited instance

* 2nd section

* docs and linting

* removing questionable `@[simp]s`

* removing away

* adding lemmas, removing 'include'

* removing import

* responding to PR comments

* use eq_of_eq to prove comp_eq_of_eq

* name change

* trying to update mathlib

* make lemma names consistent

* Update src/algebra/group/is_unit.lean

* Update src/algebra/group/is_unit.lean

* fix build

* documentation and minor changes

* spacing

* fix build

* applying comments

Co-authored-by: Chris Hughes <33847686+ChrisHughes24@users.noreply.github.com>
Co-authored-by: Chris Hughes <chrishughes24@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
butterthebuddha pushed a commit to butterthebuddha/mathlib that referenced this pull request May 16, 2020
…tions: lift, map (leanprover-community#2118)

* should I be changing and committing toml idk

* initial monoid loc lemmas

* responding to PR comments

* removing bad @[simp]

* inhabited instances

* remove #lint

* additive inhabited instance

* using is_unit & is_add_unit

* doc string

* remove simp

* submonoid.monoid_loc... -> submonoid.localization

* submonoid.monoid_loc... -> submonoid.localization

* generalize inhabited instance

* remove inhabited instance

* 2nd section

* docs and linting

* removing questionable `@[simp]s`

* removing away

* adding lemmas, removing 'include'

* removing import

* responding to PR comments

* use eq_of_eq to prove comp_eq_of_eq

* name change

* trying to update mathlib

* make lemma names consistent

* Update src/algebra/group/is_unit.lean

* Update src/algebra/group/is_unit.lean

* fix build

* documentation and minor changes

* spacing

* fix build

* applying comments

Co-authored-by: Chris Hughes <33847686+ChrisHughes24@users.noreply.github.com>
Co-authored-by: Chris Hughes <chrishughes24@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
cipher1024 pushed a commit to cipher1024/mathlib that referenced this pull request Mar 15, 2022
…tions: lift, map (leanprover-community#2118)

* should I be changing and committing toml idk

* initial monoid loc lemmas

* responding to PR comments

* removing bad @[simp]

* inhabited instances

* remove #lint

* additive inhabited instance

* using is_unit & is_add_unit

* doc string

* remove simp

* submonoid.monoid_loc... -> submonoid.localization

* submonoid.monoid_loc... -> submonoid.localization

* generalize inhabited instance

* remove inhabited instance

* 2nd section

* docs and linting

* removing questionable `@[simp]s`

* removing away

* adding lemmas, removing 'include'

* removing import

* responding to PR comments

* use eq_of_eq to prove comp_eq_of_eq

* name change

* trying to update mathlib

* make lemma names consistent

* Update src/algebra/group/is_unit.lean

* Update src/algebra/group/is_unit.lean

* fix build

* documentation and minor changes

* spacing

* fix build

* applying comments

Co-authored-by: Chris Hughes <33847686+ChrisHughes24@users.noreply.github.com>
Co-authored-by: Chris Hughes <chrishughes24@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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.

7 participants