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] - chore(ring_theory/ideal/local_ring): generalize to semirings #13341

Closed
wants to merge 14 commits into from

Conversation

yuma-mizuno
Copy link
Collaborator


Open in Gitpod

@yuma-mizuno yuma-mizuno added the awaiting-review The author would like community review of the PR label Apr 11, 2022
@@ -32,53 +32,43 @@ universes u v w

/-- A commutative ring is local if it has a unique maximal ideal. Note that
`local_ring` is a predicate. -/
class local_ring (R : Type u) [comm_ring R] extends nontrivial R : Prop :=
(is_local : ∀ (a : R), (is_unit a) ∨ (is_unit (1 - a)))
class local_ring (R : Type u) [comm_semiring R] extends nontrivial R : Prop :=
Copy link
Member

Choose a reason for hiding this comment

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

Can you add back the old nonunits_add? Now it is immediate from the definition, but it could be useful.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The old nonunits_add can be accessed with the same name local_ring.nonunits_add in the new definition. I would like to remark that the proof of the old nonunits_add is moved to the proof of the lemma local_of_is_unit_or_is_unit_one_sub_self, which plays the same role as the old version constructor of local_ring.

Copy link
Member

Choose a reason for hiding this comment

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

But since local_ring is a class in practice there will be no h : local_ring R, so it will b annoying to access it. Or do you see another way?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, if I understand correctly, the current local_ring.nonunits_add is intended to be used either with the fully specified name local_ring.nonunit_add or with nonunits_add followed by open local_ring (or in the namespace local_ring). With the new definition this lemma is exactly the projection of local_ring that is automatically generated when the class is defined.

In fact, nonunits_add is used in the definition of local_ring.maximal_ideal here, but there was no need to change this part in this PR.

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes, it works because the lemma I want is automatically created by Lean.

src/ring_theory/ideal/local_ring.lean Outdated Show resolved Hide resolved

namespace local_ring

variables {R : Type u} [comm_ring R] [local_ring R]
variables {R : Type u}
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 it is more readable if you put [comm_semiring R] [local_ring R] here., and use another variable S with [comm_ring S] [local_ring S].

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you for your suggestion! Although it's different from the proposed method, I have divided lemmas for comm_semiring and comm_ring into two sections. I think the order of the lemmas has become logically easier to read. What do you think about this?

src/ring_theory/ideal/local_ring.lean Outdated Show resolved Hide resolved
src/ring_theory/ideal/local_ring.lean Outdated Show resolved Hide resolved
end comm_semiring

section comm_ring
variables {R : Type u} [comm_ring R] [local_ring R]
Copy link
Member

Choose a reason for hiding this comment

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

You can move this line at the beginning of the file, so you only write {R : Type u} {S : Type v} once, and then you only need [comm_ring] and similar.

@riccardobrasca
Copy link
Member

There are still a lot of [semiring R] floating around. It would be better to move all the lemmas in the appropriate section. Essentially almost no lemma should have [ring R]/[semiring R] written explicitly (there are exceptions of course).

universes u v w
universes u v w u'

variables {R : Type u} {S : Type v} {T : Type w} {K : Type u'}

/-- A commutative ring is local if it has a unique maximal ideal. Note that
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be fixed.

lemma is_unit_of_mem_nonunits_one_sub_self (a : R) (h : (1 - a) ∈ nonunits R) :
is_unit a :=
or_iff_not_imp_right.1 (is_local a) h
lemma of_nonunits_ideal (hnze : (0:R) ≠ 1) (h : ∀ x y ∈ nonunits R, x + y ∈ nonunits R) :
Copy link
Member

Choose a reason for hiding this comment

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

Instead of hnze I would use nontrivial R.


end local_ring

end comm_ring

/-- A local ring homomorphism is a homomorphism between local rings
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be fixed.

@@ -32,8 +32,7 @@ universes u v w u'

variables {R : Type u} {S : Type v} {T : Type w} {K : Type u'}

/-- A commutative ring is local if it has a unique maximal ideal. Note that
`local_ring` is a predicate. -/
/-- A semiring is local if it has a unique maximal ideal. Note that `local_ring` is a predicate. -/
Copy link
Member

Choose a reason for hiding this comment

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

I mean that now the definition is not that there is only one maximal ideal.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks I see what you mean.

Having unique maximal ideal ii not the definition itself even in the previous file. I guess that the previous author wrote such docs as the file show the equivalence of the definitions soon after. Should we write the actual definition here?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, you can add a comment saying it is equivalent to the usual one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK!

@riccardobrasca
Copy link
Member

Thanks!

bors d+

@bors
Copy link

bors bot commented Apr 15, 2022

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

@github-actions github-actions bot added delegated The PR author may merge after reviewing final suggestions. and removed awaiting-review The author would like community review of the PR labels Apr 15, 2022
@yuma-mizuno
Copy link
Collaborator Author

bors r+

@bors
Copy link

bors bot commented Apr 15, 2022

Build failed:

@yuma-mizuno
Copy link
Collaborator Author

bors r+

@bors
Copy link

bors bot commented Apr 15, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title chore(ring_theory/ideal/local_ring): generalize to semirings [Merged by Bors] - chore(ring_theory/ideal/local_ring): generalize to semirings Apr 15, 2022
@bors bors bot closed this Apr 15, 2022
@bors bors bot deleted the local-semiring branch April 15, 2022 17:08
bors bot pushed a commit that referenced this pull request Apr 21, 2022
The main ingredient of this PR is the definition of `is_localization.lift` that works for semirings. The previous definition uses `ring_hom.mk'` that essentially states that `f 0 = 0` follows from other conditions. This does not holds for semirings. Instead, this PR defines the localization of monoid with zero, and uses this to define `is_localization.lift`.

- I think definitions around `localization_with_zero_map` might be ad hoc, and any suggestions for improvement are welcome!
- I plan to further generalize the localization API for semirings. This needs generalization of other ring theory stuff such as `local_ring` and `is_domain` (generalizing `local_ring` is partially done in #13341).
bors bot pushed a commit that referenced this pull request Apr 21, 2022
The main ingredient of this PR is the definition of `is_localization.lift` that works for semirings. The previous definition uses `ring_hom.mk'` that essentially states that `f 0 = 0` follows from other conditions. This does not holds for semirings. Instead, this PR defines the localization of monoid with zero, and uses this to define `is_localization.lift`.

- I think definitions around `localization_with_zero_map` might be ad hoc, and any suggestions for improvement are welcome!
- I plan to further generalize the localization API for semirings. This needs generalization of other ring theory stuff such as `local_ring` and `is_domain` (generalizing `local_ring` is partially done in #13341).
bors bot pushed a commit that referenced this pull request Apr 21, 2022
The main ingredient of this PR is the definition of `is_localization.lift` that works for semirings. The previous definition uses `ring_hom.mk'` that essentially states that `f 0 = 0` follows from other conditions. This does not holds for semirings. Instead, this PR defines the localization of monoid with zero, and uses this to define `is_localization.lift`.

- I think definitions around `localization_with_zero_map` might be ad hoc, and any suggestions for improvement are welcome!
- I plan to further generalize the localization API for semirings. This needs generalization of other ring theory stuff such as `local_ring` and `is_domain` (generalizing `local_ring` is partially done in #13341).
bors bot pushed a commit that referenced this pull request Apr 21, 2022
The main ingredient of this PR is the definition of `is_localization.lift` that works for semirings. The previous definition uses `ring_hom.mk'` that essentially states that `f 0 = 0` follows from other conditions. This does not holds for semirings. Instead, this PR defines the localization of monoid with zero, and uses this to define `is_localization.lift`.

- I think definitions around `localization_with_zero_map` might be ad hoc, and any suggestions for improvement are welcome!
- I plan to further generalize the localization API for semirings. This needs generalization of other ring theory stuff such as `local_ring` and `is_domain` (generalizing `local_ring` is partially done in #13341).
bors bot pushed a commit that referenced this pull request Apr 21, 2022
The main ingredient of this PR is the definition of `is_localization.lift` that works for semirings. The previous definition uses `ring_hom.mk'` that essentially states that `f 0 = 0` follows from other conditions. This does not holds for semirings. Instead, this PR defines the localization of monoid with zero, and uses this to define `is_localization.lift`.

- I think definitions around `localization_with_zero_map` might be ad hoc, and any suggestions for improvement are welcome!
- I plan to further generalize the localization API for semirings. This needs generalization of other ring theory stuff such as `local_ring` and `is_domain` (generalizing `local_ring` is partially done in #13341).
bors bot pushed a commit that referenced this pull request Apr 22, 2022
The main ingredient of this PR is the definition of `is_localization.lift` that works for semirings. The previous definition uses `ring_hom.mk'` that essentially states that `f 0 = 0` follows from other conditions. This does not holds for semirings. Instead, this PR defines the localization of monoid with zero, and uses this to define `is_localization.lift`.

- I think definitions around `localization_with_zero_map` might be ad hoc, and any suggestions for improvement are welcome!
- I plan to further generalize the localization API for semirings. This needs generalization of other ring theory stuff such as `local_ring` and `is_domain` (generalizing `local_ring` is partially done in #13341).
bors bot pushed a commit that referenced this pull request Apr 22, 2022
The main ingredient of this PR is the definition of `is_localization.lift` that works for semirings. The previous definition uses `ring_hom.mk'` that essentially states that `f 0 = 0` follows from other conditions. This does not holds for semirings. Instead, this PR defines the localization of monoid with zero, and uses this to define `is_localization.lift`.

- I think definitions around `localization_with_zero_map` might be ad hoc, and any suggestions for improvement are welcome!
- I plan to further generalize the localization API for semirings. This needs generalization of other ring theory stuff such as `local_ring` and `is_domain` (generalizing `local_ring` is partially done in #13341).
bors bot pushed a commit that referenced this pull request Apr 22, 2022
The main ingredient of this PR is the definition of `is_localization.lift` that works for semirings. The previous definition uses `ring_hom.mk'` that essentially states that `f 0 = 0` follows from other conditions. This does not holds for semirings. Instead, this PR defines the localization of monoid with zero, and uses this to define `is_localization.lift`.

- I think definitions around `localization_with_zero_map` might be ad hoc, and any suggestions for improvement are welcome!
- I plan to further generalize the localization API for semirings. This needs generalization of other ring theory stuff such as `local_ring` and `is_domain` (generalizing `local_ring` is partially done in #13341).
bors bot pushed a commit that referenced this pull request Apr 22, 2022
The main ingredient of this PR is the definition of `is_localization.lift` that works for semirings. The previous definition uses `ring_hom.mk'` that essentially states that `f 0 = 0` follows from other conditions. This does not holds for semirings. Instead, this PR defines the localization of monoid with zero, and uses this to define `is_localization.lift`.

- I think definitions around `localization_with_zero_map` might be ad hoc, and any suggestions for improvement are welcome!
- I plan to further generalize the localization API for semirings. This needs generalization of other ring theory stuff such as `local_ring` and `is_domain` (generalizing `local_ring` is partially done in #13341).
bors bot pushed a commit that referenced this pull request Apr 22, 2022
The main ingredient of this PR is the definition of `is_localization.lift` that works for semirings. The previous definition uses `ring_hom.mk'` that essentially states that `f 0 = 0` follows from other conditions. This does not holds for semirings. Instead, this PR defines the localization of monoid with zero, and uses this to define `is_localization.lift`.

- I think definitions around `localization_with_zero_map` might be ad hoc, and any suggestions for improvement are welcome!
- I plan to further generalize the localization API for semirings. This needs generalization of other ring theory stuff such as `local_ring` and `is_domain` (generalizing `local_ring` is partially done in #13341).
bors bot pushed a commit that referenced this pull request Apr 22, 2022
The main ingredient of this PR is the definition of `is_localization.lift` that works for semirings. The previous definition uses `ring_hom.mk'` that essentially states that `f 0 = 0` follows from other conditions. This does not holds for semirings. Instead, this PR defines the localization of monoid with zero, and uses this to define `is_localization.lift`.

- I think definitions around `localization_with_zero_map` might be ad hoc, and any suggestions for improvement are welcome!
- I plan to further generalize the localization API for semirings. This needs generalization of other ring theory stuff such as `local_ring` and `is_domain` (generalizing `local_ring` is partially done in #13341).
bors bot pushed a commit that referenced this pull request Apr 22, 2022
The main ingredient of this PR is the definition of `is_localization.lift` that works for semirings. The previous definition uses `ring_hom.mk'` that essentially states that `f 0 = 0` follows from other conditions. This does not holds for semirings. Instead, this PR defines the localization of monoid with zero, and uses this to define `is_localization.lift`.

- I think definitions around `localization_with_zero_map` might be ad hoc, and any suggestions for improvement are welcome!
- I plan to further generalize the localization API for semirings. This needs generalization of other ring theory stuff such as `local_ring` and `is_domain` (generalizing `local_ring` is partially done in #13341).
bors bot pushed a commit that referenced this pull request Apr 23, 2022
The main ingredient of this PR is the definition of `is_localization.lift` that works for semirings. The previous definition uses `ring_hom.mk'` that essentially states that `f 0 = 0` follows from other conditions. This does not holds for semirings. Instead, this PR defines the localization of monoid with zero, and uses this to define `is_localization.lift`.

- I think definitions around `localization_with_zero_map` might be ad hoc, and any suggestions for improvement are welcome!
- I plan to further generalize the localization API for semirings. This needs generalization of other ring theory stuff such as `local_ring` and `is_domain` (generalizing `local_ring` is partially done in #13341).
bors bot pushed a commit that referenced this pull request Apr 29, 2022
The main ingredient of this PR is the definition of `is_localization.lift` that works for semirings. The previous definition uses `ring_hom.mk'` that essentially states that `f 0 = 0` follows from other conditions. This does not holds for semirings. Instead, this PR defines the localization of monoid with zero, and uses this to define `is_localization.lift`.

- I think definitions around `localization_with_zero_map` might be ad hoc, and any suggestions for improvement are welcome!
- I plan to further generalize the localization API for semirings. This needs generalization of other ring theory stuff such as `local_ring` and `is_domain` (generalizing `local_ring` is partially done in #13341).
bors bot pushed a commit that referenced this pull request May 4, 2022
…13471)

What is essentially new is the proof of `local_ring.of_surjective` and `local_ring.is_unit_or_is_unit_of_is_unit_add`.

- I changed the definition of local ring to `local_ring.of_is_unit_or_is_unit_of_add_one`, which is reminiscent of the definition before the recent change in #13341. The equivalence of the previous definition is essentially given by `local_ring.is_unit_or_is_unit_of_is_unit_add`. The choice of the definition is insignificant here because they are all equivalent, but I think the choice here is better for the default constructor because this condition is "weaker" than e.g. `local_ring.of_non_units_add` in some sense.

- The proof of `local_ring.of_surjective` needs `[is_local_ring_hom f]`, which was not necessary for commutative rings in the previous proof. So the new version here is not a genuine generalization of the previous version. The previous version was  renamed to `local_ring.of_surjective'`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated The PR author may merge after reviewing final suggestions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants