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(cyclotomic/eval): (q - 1) ^ totient n < |ϕₙ(q)| #12595

Closed
wants to merge 32 commits into from

Conversation

Co-authored-by: Johan Commelin <johan@commelin.net>
@ericrbg ericrbg added awaiting-CI The author would like to see what CI has to say before doing more work. awaiting-review The author would like community review of the PR labels Mar 11, 2022
@riccardobrasca
Copy link
Member

I don't remember the details, but I have the impression that with this result you can golf a little bit the proof of nat.exists_prime_ge_modeq_one (at the beginning we need that eval _ cyclotomic is bigger then 1 or something like that).

Also, no problem the the requested review, but can you erase your comment from the PR description? I think that description ends up in the "official" Zulip stream.

@ericrbg
Copy link
Collaborator Author

ericrbg commented Mar 11, 2022

ah, I'll have a look.

and no, the "top" bit I think is what ends up on the PR description, as far as I understand the bit below the line is just for discussion whilst writing the PR

@riccardobrasca
Copy link
Member

ah, I'll have a look.

and no, the "top" bit I think is what ends up on the PR description, as far as I understand the bit below the line is just for discussion whilst writing the PR

That's very possible.

@@ -160,4 +160,52 @@ begin
exact nat.pow_right_injective hp.two_le hxy }
end

lemma sub_one_lt_nat_abs_cyclotomic_eval (n : ℕ) (q : ℕ) (hn' : 1 < n) (hq' : q ≠ 1) :
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 some comments explaining the proof? It doesn't look trivial

ring_hom.eq_int_cast, eval_sub],
rw [←finset.prod_sdiff (finset.singleton_subset_iff.mpr $ (mem_primitive_roots hn).mpr hζ),
finset.prod_singleton, ←one_mul (↑(q - 1) : nnreal)],
have aux : 1 ≤ ∏ (x : ℂ) in primitive_roots n ℂ \ {ζ}, ∥↑q - x∥₊,
Copy link
Member

Choose a reason for hiding this comment

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

This can be extracted as a separate lemma I think.

@github-actions github-actions bot removed the awaiting-CI The author would like to see what CI has to say before doing more work. label Mar 11, 2022
@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 Mar 12, 2022
@alexjbest
Copy link
Member

alexjbest commented Mar 13, 2022

I think there is a potentially more useful stronger version of this result, where we instead get (q - 1) ^ totient n < (cyclotomic n ℝ).eval q (for any real > 1). The proof is somewhat the same so I've been modifying whats here to give that result instead as it seems nice to get the best possible version. (there is an upper bound version too)

I know that this stronger version is used in a proof of Zsigmondy's theorem for instance.

Would you mind waiting a day or two until thats done, and then I'll push it here?

@ericrbg
Copy link
Collaborator Author

ericrbg commented Mar 13, 2022

Sure, feel free to push and I'll fix it up then. I was busy this weekend anyways so it all works out ^^

@alexjbest
Copy link
Member

Ok I've pushed where I'm up to, its sorry free, but a bit of a monster proof. I really had to fight super hard to apply prod_lt_prod' as it needs an ordered cancel monoid, so I had to move everything to positive reals (by taking the units of nnreal). It makes me wonder if we need some other versions of that theorem that are easier to apply, but I'm not sure how.

Anyone should feel free to fix up my bad work, I'll keep chipping away at it too. I just didn't want to hold the branch hostage any longer ;).

Note that I added the statement for the upper bound too, I can't see a sneaky way to reuse the same proof, or use one to prove the other (note the upper bound has a stronger condition on n). So maybe we can just copy paste when the proof is minimized a bit.

@alexjbest alexjbest changed the title feat(cyclotomic/eval): q - 1 < |ϕₙ(q)| feat(cyclotomic/eval): (q - 1) ^ totient n < |ϕₙ(q)| Mar 14, 2022
@ericrbg ericrbg added the WIP Work in progress label Mar 15, 2022
@leanprover-community-bot-assistant leanprover-community-bot-assistant 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 Apr 25, 2022
@ericrbg ericrbg 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 awaiting-author A reviewer has asked the author a question or requested changes labels Apr 26, 2022
@github-actions github-actions bot removed the awaiting-CI The author would like to see what CI has to say before doing more work. label Apr 26, 2022
@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Apr 26, 2022
@leanprover-community-bot-assistant leanprover-community-bot-assistant removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Apr 27, 2022
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 have a quick look at the proof of nat.exists_prime_ge_modeq_one? It can maybe be golfed using this (and one variable should be made implicit)

src/ring_theory/polynomial/cyclotomic/eval.lean Outdated Show resolved Hide resolved
src/ring_theory/polynomial/cyclotomic/eval.lean Outdated Show resolved Hide resolved
src/ring_theory/polynomial/cyclotomic/eval.lean Outdated Show resolved Hide resolved
ericrbg and others added 2 commits April 28, 2022 13:47
Co-authored-by: Riccardo Brasca <riccardo.brasca@gmail.com>
@ericrbg
Copy link
Collaborator Author

ericrbg commented Apr 28, 2022

If it can, it won't be too huge of a golf - because these theorems require 1 < k (for k being the number of the cyclotomic poly) but yours needs 0 < n. (it is of course a ≤ for all 0 < k, should I add this version too?)

@riccardobrasca
Copy link
Member

That's not so important, don't worry.

@riccardobrasca
Copy link
Member

Let's just get this merged, thanks!

bors d+

@bors
Copy link

bors bot commented Apr 28, 2022

✌️ ericrbg 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 28, 2022
@ericrbg
Copy link
Collaborator Author

ericrbg commented Apr 28, 2022

bors r+

bors bot pushed a commit that referenced this pull request Apr 28, 2022
Originally from the Wedderburn PR, but generalized to include an exponent.

Co-authored-by: Alex J. Best <alex.j.best@gmail.com>



Co-authored-by: Alex J. Best <alex.j.best@gmail.com>
Co-authored-by: Johan Commelin <johan@commelin.net>
Co-authored-by: Alex J Best <alex.j.best@gmail.com>
@bors
Copy link

bors bot commented Apr 28, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(cyclotomic/eval): (q - 1) ^ totient n < |ϕₙ(q)| [Merged by Bors] - feat(cyclotomic/eval): (q - 1) ^ totient n < |ϕₙ(q)| Apr 28, 2022
@bors bors bot closed this Apr 28, 2022
@bors bors bot deleted the lt_eval_cyclotomic branch April 28, 2022 20:22
bors bot pushed a commit that referenced this pull request Apr 29, 2022
As suggested in the reviews of #12595 we try to golf the proof using the bound proved there.
This doesn't end up being as much of a golf as hoped due to annoying edge cases, but seems conceptually simpler.
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

5 participants