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] - refactor(ring_theory/ideal/*, ring_theory/jacobson): use comm_semiring instead of comm_ring for ideals #5954

Closed
wants to merge 16 commits into from

Conversation

adomani
Copy link
Collaborator

@adomani adomani commented Jan 29, 2021

This is the second pass at refactoring the definition of ideal. I have changed a comm_ring assumption to a comm_semiring assumption on many of the lemmas in ring_theory/ideal/basic. Most implied changes were very simple, with the usual exception of jacobson.

I also moved out of jacobson the lemmas that were left-over from the previous refactor in this sequence.

Besides changing such assumptions on other files, many of the lemmas in ring_theory/ideal/basic still using comm_ring and without explicit subtractions, deal with quotients.


@adomani
Copy link
Collaborator Author

adomani commented Jan 29, 2021

@bryangingechen bryangingechen added the awaiting-review The author would like community review of the PR label Jan 29, 2021
adomani and others added 2 commits January 29, 2021 18:21
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
@adomani
Copy link
Collaborator Author

adomani commented Jan 29, 2021

Eric, thank you very much for your suggested improvements!

Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
@jcommelin jcommelin changed the title refactor(ring_theory/ideal/ideal/basic + over and ring_theory/jacobson ): use comm_semiring instead of comm_ring for ideals refactor(ring_theory/ideal/*, ring_theory/jacobson): use comm_semiring instead of comm_ring for ideals Jan 29, 2021
adomani and others added 2 commits January 29, 2021 19:58
Co-authored-by: Xavier Xarles <56635243+XavierXarles@users.noreply.github.com>
Co-authored-by: Xavier Xarles <56635243+XavierXarles@users.noreply.github.com>
@adomani
Copy link
Collaborator Author

adomani commented Jan 29, 2021

Xavier, thank you for your improvements!

Copy link
Collaborator

@bryangingechen bryangingechen left a comment

Choose a reason for hiding this comment

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

Some very minor formatting nitpicks.

I haven't seen any objections, so feel free to merge.
bors d+

src/ring_theory/jacobson_ideal.lean Outdated Show resolved Hide resolved
src/ring_theory/jacobson.lean Outdated Show resolved Hide resolved
src/ring_theory/ideal/basic.lean Outdated Show resolved Hide resolved
@bors
Copy link

bors bot commented Jan 31, 2021

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

@bryangingechen bryangingechen added awaiting-author A reviewer has asked the author a question or requested changes delegated The PR author may merge after reviewing final suggestions. and removed awaiting-review The author would like community review of the PR labels Jan 31, 2021
@@ -84,16 +78,6 @@ theorem eq_top_iff_one : I = ⊤ ↔ (1:α) ∈ I :=
theorem ne_top_iff_one : I ≠ ⊤ ↔ (1:α) ∉ I :=
not_congr I.eq_top_iff_one

lemma exists_mem_ne_zero_iff_ne_bot : (∃ p ∈ I, p ≠ (0 : α)) ↔ I ≠ ⊥ :=
Copy link
Member

Choose a reason for hiding this comment

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

At a glance, I'm surprised this isn't true for semirings

Copy link
Collaborator Author

@adomani adomani Feb 1, 2021

Choose a reason for hiding this comment

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

Eric, you are right: below is a proof (the first one that I found) of the "same" result for a submodule of R, when R is a semiring.

However, at the moment, ideal is only defined for a comm_semiring. In fact, I think that what you suggest should be a separate PR, in case it is not already there.

More generally, we should have a "theory" of cyclic modules, i.e. modules generated by a single element, where this result would fit nicely, replacing 1 : R by a generating element of the module.

lemma submodule.ne_top_iff_one {R : Type*} [semiring R]
  (J : submodule R R): J ≠ ⊤ ↔ (1:R) ∉ J :=
not_congr ⟨λ a, submodule.eq_top_iff'.mp a 1, λ hx, submodule.ext
  (λ x, ⟨λ _, submodule.mem_top, λ xj, by { rw [← mul_one x], exact submodule.smul_mem J x hx }⟩)⟩

Co-authored-by: Bryan Gin-ge Chen <bryangingechen@gmail.com>
@adomani
Copy link
Collaborator Author

adomani commented Feb 1, 2021

bors r+

@adomani
Copy link
Collaborator Author

adomani commented Feb 1, 2021

Thanks Bryan, I just told Bors to merge!

Eric, you have a point with the semiring issue, but I will leave this for another moment, when refactoring to eliminate commutativity. Does this sound alright?

bors bot pushed a commit that referenced this pull request Feb 1, 2021
…ng` instead of `comm_ring` for ideals (#5954)

This is the second pass at refactoring the definition of `ideal`.  I have changed a `comm_ring` assumption to a `comm_semiring` assumption on many of the lemmas in `ring_theory/ideal/basic`.  Most implied changes were very simple, with the usual exception of `jacobson`.

I also moved out of `jacobson` the lemmas that were left-over from the previous refactor in this sequence.

Besides changing such assumptions on other files, many of the lemmas in `ring_theory/ideal/basic` still using `comm_ring` and without explicit subtractions, deal with quotients.
@bors
Copy link

bors bot commented Feb 2, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title refactor(ring_theory/ideal/*, ring_theory/jacobson): use comm_semiring instead of comm_ring for ideals [Merged by Bors] - refactor(ring_theory/ideal/*, ring_theory/jacobson): use comm_semiring instead of comm_ring for ideals Feb 2, 2021
@bors bors bot closed this Feb 2, 2021
@bors bors bot deleted the semi_ideals branch February 2, 2021 02:06
b-mehta pushed a commit that referenced this pull request Apr 2, 2021
…ng` instead of `comm_ring` for ideals (#5954)

This is the second pass at refactoring the definition of `ideal`.  I have changed a `comm_ring` assumption to a `comm_semiring` assumption on many of the lemmas in `ring_theory/ideal/basic`.  Most implied changes were very simple, with the usual exception of `jacobson`.

I also moved out of `jacobson` the lemmas that were left-over from the previous refactor in this sequence.

Besides changing such assumptions on other files, many of the lemmas in `ring_theory/ideal/basic` still using `comm_ring` and without explicit subtractions, deal with quotients.
@adomani adomani removed the awaiting-author A reviewer has asked the author a question or requested changes label Jun 15, 2021
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

4 participants