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(ring_theory/*): generalise minpoly.dvd to integrally closed rings #18021

Closed
wants to merge 47 commits into from

Conversation

Paul-Lez
Copy link
Collaborator

@Paul-Lez Paul-Lez commented Dec 29, 2022

This PR generalizes some of the theory of minpoly to the setting of integrally closed rings. The main results proven here are:

  • minpoly.is_integrally_closed_eq_field_fractions and variants of it
  • minpoly.is_integrally_closed_dvd
  • monic.dvd_of_fraction_map_dvd_fraction_map

In a following PR, I will remove instances of gcd_domain_dvd (and the other results this PR generalises) from other files, and replace the file minpoly.gcd_domain by minpoly.is_integrally_closed

Co-authored-by: Junyan Xu junyanxu.math@gmail.com


Open in Gitpod

@Paul-Lez Paul-Lez added WIP Work in progress t-algebra Algebra (groups, rings, fields etc) labels Dec 29, 2022
@Paul-Lez Paul-Lez added awaiting-review The author would like community review of the PR and removed WIP Work in progress labels Dec 29, 2022
@Paul-Lez
Copy link
Collaborator Author

This PR is now ready for review. If someone has some advice on where the main results could be put, that would be super helpful.

Copy link
Collaborator

@ericrbg ericrbg left a comment

Choose a reason for hiding this comment

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

some random stuff I saw - I'm too tired but I'll keep reviewing in the morning. it seems a lot of the results you have are strictly stronger than some already-existing ones so I'd just replace those in-place.

src/number_theory/temporary_file.lean Outdated Show resolved Hide resolved
src/number_theory/temporary_file.lean Outdated Show resolved Hide resolved
src/number_theory/temporary_file.lean Outdated Show resolved Hide resolved
src/number_theory/temporary_file.lean Outdated Show resolved Hide resolved
src/number_theory/temporary_file.lean Outdated Show resolved Hide resolved
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.

Can you please fix all the linter errors and change the name of temporary_file file?

We still don't know that a gcd_monoid is integrally closed, right?

@riccardobrasca riccardobrasca 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 Dec 30, 2022
@riccardobrasca
Copy link
Member

@riccardobrasca, leanprover-community.github.io/mathlib_docs/algebra/gcd_monoid/integrally_closed.html

Good! So there should be at least two or three results that can be generalized to integrally closed rings.

@Paul-Lez Paul-Lez added awaiting-review The author would like community review of the PR WIP Work in progress and removed awaiting-author A reviewer has asked the author a question or requested changes awaiting-review The author would like community review of the PR labels Dec 30, 2022
src/data/polynomial/algebra_map.lean Outdated Show resolved Hide resolved
@@ -125,6 +125,19 @@ begin
aeval_alg_hom_apply, aeval_map_algebra_map, aeval_def, hP.2, _root_.map_zero]
end

lemma is_integral_of_commuting_diagram {R S T U : Type*} [nontrivial S] [nontrivial T]
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as above about the name. This can maybe be golfed introducing some is_scalar_tower, but I am not sure.

Copy link
Collaborator Author

@Paul-Lez Paul-Lez Jan 4, 2023

Choose a reason for hiding this comment

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

I guess using is_scalar_tower is equivalent to this, since we have ring_hom.to_algebra. Which one would you think is most useful/practical to use?

src/ring_theory/integral_closure.lean Outdated Show resolved Hide resolved
src/ring_theory/localization/fraction_ring.lean Outdated Show resolved Hide resolved
src/ring_theory/polynomial/gauss_lemma.lean Outdated Show resolved Hide resolved
src/ring_theory/polynomial/gauss_lemma.lean Outdated Show resolved Hide resolved
src/ring_theory/polynomial/gauss_lemma.lean Outdated Show resolved Hide resolved
src/number_theory/temporary_file.lean Outdated Show resolved Hide resolved
src/number_theory/temporary_file.lean Outdated Show resolved Hide resolved
bors bot pushed a commit that referenced this pull request Jan 3, 2023
…hree files (#18048)

In this PR, we split the results contained in `minpoly.lean` into three separate files: `minpoly.basic.lean`, `minpoly.field.lean` and `minpoly.gcd_monoid.lean`. This is helpful for PR #18021.
bors bot pushed a commit that referenced this pull request Jan 4, 2023
…hree files (#18048)

In this PR, we split the results contained in `minpoly.lean` into three separate files: `minpoly.basic.lean`, `minpoly.field.lean` and `minpoly.gcd_monoid.lean`. This is helpful for PR #18021.
@Paul-Lez Paul-Lez added the WIP Work in progress label Jan 11, 2023
@riccardobrasca
Copy link
Member

Actually I don't understand why you didn't generalize polynomial.is_primitive.irreducible_iff_irreducible_map_fraction_map to integrally closed domains (restricting to monic polynomials). Then gcd_domain_eq_field_fractions can be generalized to integrally closed domains, keeping essentially the same proof. In practice this gives is_integrally_closed_map_minpoly_eq_minpoly_fraction_ring with only is_domain S.
In any case note the inconsistency between the names of gcd_domain_eq_field_fractions and is_integrally_closed_map_minpoly_eq_minpoly_fraction_ring.

I hadn't really thought about that - I think what happened was I already had a proof of is_integrally_closed_map_minpoly_eq_minpoly_fraction_ring following the approach @alreadydone has suggested on Zulip, and realized I could generalize some of the results from gauss_lemma.lean to integrally closed domains only later. In any case I'll have a go at what you're suggesting!

I didn't check carefully, but it seems that the result about the minimal polynomial follow by the fact that irreducible over K is the same as irreducible over R. This is currently proved for primitive polynomials over gcd_monoid, but the minimal polynomial is always monic and for monic polynomials this is true over integrally closed domains (but we still want the version for primitive, that is more general, even if the assumption on R is stronger).

I think that the confusion is that there are two things people call "Gauss lemma":

  • the product of primitive polynomials is primitive: this is what we have for gcd_monoid, and I believe the strongest version is for UFD (it should be equivalent to being a UFD);
  • irreducibility/divisibility over R is the same as irreducibility/divisibility over K for monic polynomials. This follows from the first, but it is also true over integrally closed domains (it is equivalent to being integrally closed I believe).

The theory of minimal polynomial needed the second one.

@mathlib-dependent-issues-bot mathlib-dependent-issues-bot added the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Jan 13, 2023
bors bot pushed a commit that referenced this pull request Jan 14, 2023
…egrally closed rings (#18147)

In this PR, we prove Gauss's lemma for integrally closed rings. See #18021 and #11523 for previous discussion on the topic.
We also show that integrally closed domains are precisely the domains in which Gauss's lemma holds for monic polynomials. 

[Zulip discussion](https://leanprover.zulipchat.com/#narrow/stream/144837-PR-reviews/topic/.2318021.20generalizing.20theory.20of.20minpoly)

Co-authored-by: Junyan Xu <junyanxu.math@gmail.com>




Co-authored-by: Paul Lezeau <paul.lezeau@gmail.com>
@mathlib-dependent-issues-bot mathlib-dependent-issues-bot removed the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Jan 14, 2023
@mathlib-dependent-issues-bot
Copy link
Collaborator

@riccardobrasca
Copy link
Member

Not that the other PR has been merged, can you summarize what is done in this one? For example, I guess the description needs to be updated.

@Paul-Lez
Copy link
Collaborator Author

Not that the other PR has been merged, can you summarize what is done in this one? For example, I guess the description needs to be updated.

Sure ! I'll first update the code - I think some of the lemmas I had written for the original proofs are no longer necessary for instance - and then update the summary.

@Paul-Lez Paul-Lez added awaiting-review The author would like community review of the PR awaiting-CI The author would like to see what CI has to say before doing more work. and removed WIP Work in progress labels Jan 17, 2023
src/data/polynomial/algebra_map.lean Show resolved Hide resolved
exact (lt_iff_not_ge _ _ ).mp h₂ (degree_le_of_dvd h₁ hc),
end

lemma eq_zero_of_nat_degree_lt {p q : R[X]} (h₁ : p ∣ q) (h₂ : nat_degree q < nat_degree p) :
Copy link
Member

Choose a reason for hiding this comment

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

If I am not confused by the implications, you can use this to generalize polynomial.monic.not_dvd_of_degree_lt removing the monic assumption. In doing so you should change the name of polynomial.monic.not_dvd_of_degree_lt, if this means modifying a lot of files where it is used can you please open another PR and asking for my review? It can be merged very quickly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, I'll try and have a go at that later on today!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Haha polynomial.monic.not_dvd_of_degree_lt was added by me days ago, so not many things depend on it. It holds for any semiring without need of no_zero_divisor though. Since this section has no_zero_divisors present, you can indeed prove a version without monic here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point! Well I've just added the other version without monic - it's probably good to have both anyway

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But I guess in that case no need to remove polynomial.monic.not_dvd_of_degree_lt from the files that use it :)

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I didn't realize they had orthogonal assumptions.

src/field_theory/minpoly/gcd_monoid.lean Outdated Show resolved Hide resolved
@riccardobrasca
Copy link
Member

Thanks!

bors d+

@bors
Copy link

bors bot commented Jan 17, 2023

✌️ Paul-Lez 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 awaiting-CI The author would like to see what CI has to say before doing more work. labels Jan 17, 2023
@Paul-Lez
Copy link
Collaborator Author

bors merge

bors bot pushed a commit that referenced this pull request Jan 17, 2023
…ngs (#18021)

This PR generalizes some of the theory of `minpoly` to the setting of integrally closed rings. The main results proven here are: 
 - `minpoly.is_integrally_closed_eq_field_fractions` and variants of it 
 - `minpoly.is_integrally_closed_dvd`
 - `monic.dvd_of_fraction_map_dvd_fraction_map`

In a following PR, I will remove instances of `gcd_domain_dvd` (and the other results this PR generalises) from other files, and replace the file `minpoly.gcd_domain` by `minpoly.is_integrally_closed`

Co-authored-by: Junyan Xu <junyanxu.math@gmail.com>



Co-authored-by: Paul Lezeau <paul.lezeau@gmail.com>
@bors
Copy link

bors bot commented Jan 18, 2023

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(ring_theory/*): generalise minpoly.dvd to integrally closed rings [Merged by Bors] - feat(ring_theory/*): generalise minpoly.dvd to integrally closed rings Jan 18, 2023
@bors bors bot closed this Jan 18, 2023
@bors bors bot deleted the preliminary_ring_theory_PR branch January 18, 2023 01:35
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. t-algebra Algebra (groups, rings, fields etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants