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(order/ideal, order/pfilter, order/prime_ideal): added ideal_inter_nonempty, proved that a maximal ideal is prime #6924

Closed
wants to merge 73 commits into from

Conversation

atarnoam
Copy link
Collaborator

@atarnoam atarnoam commented Mar 28, 2021

  • ideal_inter_nonempty: the intersection of any two ideals is nonempty. A preorder with joins and this property satisfies that its ideal poset is a lattice.
  • An ideal is prime iff x \inf y \in I implies x \in I or y \in I.
  • A maximal ideal in a distributive lattice is prime.

Open in Gitpod

While proving that a maximal ideal in a distributive lattice is prime, I've noticed a problem.
I wanted to use the sup of two ideals, however in the current implementation this requires P to have a bottom element!
What is going on here is that to prove that the sup ideal is nonempty, it suffices to use only the fact that the intersection of any two ideals is nonempty. This is true for both the case where P has bot, and for when P has infs.
Thus we can define this property separately and show that it is true for these two cases.

atarnoam and others added 30 commits March 12, 2021 16:42
Co-authored-by: Bryan Gin-ge Chen <bryangingechen@gmail.com>
Co-authored-by: Bryan Gin-ge Chen <bryangingechen@gmail.com>
Co-authored-by: Bryan Gin-ge Chen <bryangingechen@gmail.com>
Co-authored-by: Bryan Gin-ge Chen <bryangingechen@gmail.com>
Co-authored-by: Bryan Gin-ge Chen <bryangingechen@gmail.com>
Co-authored-by: Bryan Gin-ge Chen <bryangingechen@gmail.com>
Co-authored-by: Bryan Gin-ge Chen <bryangingechen@gmail.com>
Co-authored-by: Mathieu Guay-Paquet <mathieu.guaypaquet@gmail.com>
To show that `ideal P` is a lattice, it now suffices
to check that `P` has joins
and that the
intersection of any two ideals is nonempty.

Also added an equivalent
characteristic of the join ideal
when `P` is a lattice.
Co-authored-by: Bryan Gin-ge Chen <bryangingechen@gmail.com>
Co-authored-by: Bryan Gin-ge Chen <bryangingechen@gmail.com>
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.

This is not really my area of expertise. So I'm just commenting on style.

src/order/ideal.lean Outdated Show resolved Hide resolved
src/order/pfilter.lean Outdated Show resolved Hide resolved
src/order/ideal.lean Outdated Show resolved Hide resolved
@jcommelin jcommelin 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 Apr 6, 2021
atarnoam and others added 2 commits April 8, 2021 21:43
Co-authored-by: Johan Commelin <johan@commelin.net>
@atarnoam atarnoam 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 Apr 8, 2021
Copy link
Collaborator

@mguaypaq mguaypaq 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 in reviewing, and thank you for this nice addition. I especially like your ideal_inter_nonempty, because I've wanted something like it before. As you noted, there are two natural and unrelated settings where this property holds (the two instances of it you define), and I don't think there's a better abstraction which covers both.

As always, I've tried to be somewhat thorough, so this review includes many comments, but most of them are minor changes due to the mathlib naming conventions and style guide.

src/order/ideal.lean Outdated Show resolved Hide resolved
src/order/ideal.lean Outdated Show resolved Hide resolved
src/order/ideal.lean Outdated Show resolved Hide resolved
src/order/ideal.lean Outdated Show resolved Hide resolved
src/order/ideal.lean Outdated Show resolved Hide resolved
src/order/prime_ideal.lean Outdated Show resolved Hide resolved
src/order/prime_ideal.lean Outdated Show resolved Hide resolved
src/order/prime_ideal.lean Outdated Show resolved Hide resolved
src/order/prime_ideal.lean Outdated Show resolved Hide resolved
variables {I : ideal P}

@[priority 100]
instance is_maximal.is_prime [is_maximal I] : is_prime I :=
Copy link
Collaborator

Choose a reason for hiding this comment

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

For such a nice theorem, and the main point of this PR, it's almost too bad that the statement is so unassuming!

@mguaypaq mguaypaq 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 Apr 11, 2021
@atarnoam atarnoam 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 Apr 11, 2021
Copy link
Collaborator

@mguaypaq mguaypaq left a comment

Choose a reason for hiding this comment

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

This PR seems fine to me, but a maintainer will have to poke bors so it can be merged.

@mguaypaq mguaypaq 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 12, 2021
@awainverse awainverse added awaiting-review The author would like community review of the PR and removed ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) labels Apr 12, 2021
@awainverse
Copy link
Collaborator

I think the ready-to-merge label indicates that a maintainer has already told bors to merge it, and that it's about to merge

@bryangingechen
Copy link
Collaborator

Looks like there's just one docstring suggestion left.

By the way, @atarnoam the way we've been doing reviews here, the author usually marks suggestions / conversations as "Resolved" if they've been addressed by commits or if the conversation has ended. This makes it easier for later reviewers to see at a glance what remains to be done.

@awainverse any final thoughts on this?

Co-authored-by: Mathieu Guay-Paquet <mathieu.guaypaquet@gmail.com>
@bryangingechen
Copy link
Collaborator

Thanks!
bors r+

@github-actions github-actions bot 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 14, 2021
bors bot pushed a commit that referenced this pull request Apr 14, 2021
…er_nonempty`, proved that a maximal ideal is prime (#6924)

- `ideal_inter_nonempty`: the intersection of any two ideals is nonempty. A preorder with joins and this property satisfies that its ideal poset is a lattice.
- An ideal is prime iff `x \inf y \in I` implies `x \in I` or `y \in I`.
- A maximal ideal in a distributive lattice is prime.
@bors
Copy link

bors bot commented Apr 15, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(order/ideal, order/pfilter, order/prime_ideal): added ideal_inter_nonempty, proved that a maximal ideal is prime [Merged by Bors] - feat(order/ideal, order/pfilter, order/prime_ideal): added ideal_inter_nonempty, proved that a maximal ideal is prime Apr 15, 2021
@bors bors bot closed this Apr 15, 2021
@bors bors bot deleted the is_prime_of_is_maximal branch April 15, 2021 03:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.

None yet

5 participants