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: small lemmas about moebius #11770

Closed
wants to merge 1 commit into from

Conversation

Ruben-VandeVelde
Copy link
Collaborator


Open in Gitpod

@Ruben-VandeVelde Ruben-VandeVelde added the awaiting-review The author would like community review of the PR label Mar 29, 2024
@loefflerd
Copy link
Collaborator

LGTM

maintainer merge

Copy link

github-actions bot commented Apr 1, 2024

🚀 Pull request has been placed on the maintainer queue by loefflerd.

@@ -1073,12 +1073,17 @@ theorem moebius_ne_zero_iff_squarefree {n : ℕ} : μ n ≠ 0 ↔ Squarefree n :
· simp [h, pow_ne_zero]
#align nat.arithmetic_function.moebius_ne_zero_iff_squarefree ArithmeticFunction.moebius_ne_zero_iff_squarefree

theorem moebius_eq_or (n : ℕ) : μ n = 0 ∨ μ n = 1 ∨ μ n = -1 := by
Copy link
Member

Choose a reason for hiding this comment

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

To me this looks like an argument to change the return type to be SignType

Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this just add yet another layer of coercions that you have to translate everything through whenever you want to do arithmetic with μ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

To push this further, I guess one could argue that ArithmeticFunction.zeta should take values in Fin 2...

Copy link
Member

Choose a reason for hiding this comment

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

I suspect that the coercions aren't really a problem; and in exchange, you get results like this and the abs one for free

Copy link
Member

Choose a reason for hiding this comment

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

(if the coercions were really that bad, then presumably SignType wouldn't exist at all!)

Copy link
Collaborator

Choose a reason for hiding this comment

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

#10883. See also #10920, which was some API additions for SignType which I had to add to get #10883 working.

Copy link
Collaborator

Choose a reason for hiding this comment

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

FWIW, I just had a stab at reimplementing μ to be SignType-valued, just to check how bad it would be. It's every bit as bad as I expected. Literally every single lemma in the library about μ would need to be rewritten, with clunkier statements and longer proofs. We'd also need to add a whole load of tedious boilerplate code about coercions ArithmeticFunction SignType → ArithmeticFunction R for appropriate R, and compatibility of these with morphisms between R's. This is really, really not the way to go.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the cross-links. I'll have a think about this some more later, but actually changing this probably isn't a blocker for this PR (I'll let another maintainer decide).

Copy link
Member

Choose a reason for hiding this comment

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

FWIW, I just had a stab at reimplementing μ to be SignType-valued, just to check how bad it would be

Would you mind sharing the branch with this stab on? No need to PR it, but if you put in the work it's nice to refer back to if this comes up again in future.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I deleted it, sorry.

@MichaelStollBayreuth
Copy link
Collaborator

MichaelStollBayreuth commented Apr 1, 2024

Thanks for doing this! Once it is in, I'll use it to golf a bit in #11712 as you suggested.
(Or in a follow-up PR if this here takes longer...)

@fpvandoorn
Copy link
Member

This seems reasonable to me, and I'm not convinced that going to SignType will make things (significantly) nicer.

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review The author would like community review of the PR labels Apr 2, 2024
mathlib-bors bot pushed a commit that referenced this pull request Apr 2, 2024
@mathlib-bors
Copy link

mathlib-bors bot commented Apr 2, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat: small lemmas about moebius [Merged by Bors] - feat: small lemmas about moebius Apr 2, 2024
@mathlib-bors mathlib-bors bot closed this Apr 2, 2024
@mathlib-bors mathlib-bors bot deleted the moebius branch April 2, 2024 17:34
Louddy pushed a commit that referenced this pull request Apr 15, 2024
atarnoam pushed a commit that referenced this pull request Apr 16, 2024
uniwuni pushed a commit that referenced this pull request Apr 19, 2024
callesonne pushed a commit that referenced this pull request Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer-merge ready-to-merge This PR has been sent to bors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants