-
Notifications
You must be signed in to change notification settings - Fork 299
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(field_theory/minimal_polynomial): generalize irreducible #5006
Conversation
I think this can be golfed down with |
If More concretely, if you take I do not know the minimal assumption to have that minimal polynomials are primes (UFD? integrally closed?), but some results in the file definitely do not work over any domain. |
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Thanks, this is very useful and the code looks clean! I would like to help with de-duplicating the proof when I have some time, so I have self-requested a review to remind me (you are welcome to go ahead without me of course!). |
The proof is very linear, it is just what I would write on paper, except that I wouldn't do it for both factors. The annoying part is that by definition the minimal polynomial has minimal degree among the monic polynomials that have |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These would be my suggestions to generalize and deduplicate the code.
Co-authored-by: Anne Baanen <Vierkantor@users.noreply.github.com>
I can also add that for GCD domains the minimal polynomial over the ring is the same as the minimal polynomial over the fraction field. It should be just a few lines. |
If it's not too much work for you, I would appreciate it. Otherwise I'm happy with merging it after someone else has looked at my additions. Thanks for your work! |
I have added The statement is almost as long as the proof, but if I do not put |
I have no idea why CI fails on a file that I didn't touch... I modified |
It seems that in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two small changes and it looks good to me. Thanks 🎉
bors d+
@@ -448,7 +448,8 @@ begin | |||
(minimal_polynomial.dvd _ _), | |||
{ rw ← is_scalar_tower.algebra_map_eq, exact H2 }, | |||
{ rw [← is_scalar_tower.aeval_apply, minimal_polynomial.aeval H1] } }, | |||
obtain ⟨y, hy⟩ := polynomial.exists_root_of_splits _ H6 (minimal_polynomial.degree_ne_zero H5), | |||
obtain ⟨y, hy⟩ := polynomial.exists_root_of_splits _ H6 | |||
(ne_of_lt (minimal_polynomial.degree_pos H5)).symm, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(ne_of_lt (minimal_polynomial.degree_pos H5)).symm, | |
(ne_of_lt (minimal_polynomial.degree_pos H5)).symm, |
✌️ riccardobrasca can now approve this pull request. To approve and merge a pull request, simply reply with |
Co-authored-by: Anne Baanen <Vierkantor@users.noreply.github.com>
Co-authored-by: Anne Baanen <Vierkantor@users.noreply.github.com>
bors r+ |
I have removed the assumption that the base ring is a field for a minimal polynomial to be irreducible. The proof is simple but long, it should be possible to use `wlog` to shorten it, but I do not understand how to do it...
Pull request successfully merged into master. Build succeeded: |
I have removed the assumption that the base ring is a field for a minimal polynomial to be irreducible.
The proof is simple but long, it should be possible to use
wlog
to shorten it, but I do not understand how to do it...