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

feat(archive/imo): formalise 1979 Q1 #4956

Closed
wants to merge 24 commits into from
Closed

feat(archive/imo): formalise 1979 Q1 #4956

wants to merge 24 commits into from

Conversation

kbuzzard
Copy link
Member

@kbuzzard kbuzzard commented Nov 9, 2020


Comment on lines 98 to 118
apply finset.subset.antisymm,
{ intros x hx,
rw mem_filter at hx,
rcases hx with ⟨hx1, m, rfl⟩,
rw mem_map,
use m,
existsi _, refl,
rw mem_range at hx1 ⊢,
linarith },
{ intros x hx,
rw mem_filter,
rw mem_map at hx,
rcases hx with ⟨a, ha, rfl⟩,
split,
{ rw mem_range at ha ⊢,
change 2 * a < 1320,
linarith },
{ existsi a,
refl
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Is it really this hard to prove that filter even (range 1320) = map double (range 660)? That's awful!

Copy link
Member

Choose a reason for hiding this comment

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

I can do it with slightly fewer lines with

example : filter even (range 1320) = map double (range 660) :=
begin
  ext a,
  simp only [mem_filter, mem_range, mem_map, double],
  split,
  { rintros ⟨x, m, rfl⟩,
    refine ⟨m, _, rfl⟩,
    rw mem_range,
    linarith },
  { rintros ⟨a, ha, rfl⟩,
    refine ⟨_, a, rfl⟩,
    rw mem_range at ha,
    change 2 * a < 1320,
    linarith, }
end

Copy link
Collaborator

Choose a reason for hiding this comment

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

In this case, the general form of the lemma seems to be easier to prove

example (n : ℕ) : filter even (range (2*n)) = map double (range n) :=
begin
  induction n with k IH,
  { refl },
  { rw [mul_succ, range_succ, range_succ, range_succ, map_insert, filter_insert, if_neg, 
        filter_insert, if_pos, IH];
    simp with parity_simps }
end

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd think a more general form takes a monotone function, filters an interval on membership of the image of that function and shows the result is the image of an interval given suitable conditions on the orders involved. With special cases for arithmetic progressions, i.e. images of intervals under functions a * x + b in nat or int being equivalent to filtering intervals on some modular arithmetic condition, and versions for odd and even being special cases of that. (And most of that making sense for both set and finset.) That may not make proofs significantly simpler, but it fits in with mathlib principles, and those more general lemmas about intervals, monotone functions and arithmetic progressions seem plausibly appropriate for mathlib proper.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've gone with @eric-wieser 's proof.

Copy link
Member Author

Choose a reason for hiding this comment

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

Having said that, I now feel guilty because one of the main gains that we're getting from formalising IMO solutions is that it shows up gaps in the library...

@bryangingechen bryangingechen added awaiting-review The author would like community review of the PR imo Formalisation of an IMO problem labels Nov 9, 2020
Copy link
Collaborator

@bryangingechen bryangingechen left a comment

Choose a reason for hiding this comment

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

My suggestions are just a few style comments.

I've only glanced at the proofs so far, but their length suggests to me that we might be missing some API for manipulating sums. I don't have any concrete suggestions yet, but what was your feeling when you were writing the proofs?

archive/imo/imo1979_q1.lean Outdated Show resolved Hide resolved
archive/imo/imo1979_q1.lean Outdated Show resolved Hide resolved
archive/imo/imo1979_q1.lean Outdated Show resolved Hide resolved
archive/imo/imo1979_q1.lean Outdated Show resolved Hide resolved
archive/imo/imo1979_q1.lean Outdated Show resolved Hide resolved
archive/imo/imo1979_q1.lean Outdated Show resolved Hide resolved
archive/imo/imo1979_q1.lean Outdated Show resolved Hide resolved
archive/imo/imo1979_q1.lean Outdated Show resolved Hide resolved
archive/imo/imo1979_q1.lean Outdated Show resolved Hide resolved
archive/imo/imo1979_q1.lean Outdated Show resolved Hide resolved
@kbuzzard kbuzzard 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 Nov 11, 2020
Copy link
Member Author

@kbuzzard kbuzzard left a comment

Choose a reason for hiding this comment

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

I've come back to this and tried to tidy up all the sum proofs, and I think @jsm28 is right. There are some sum lemmas missing in finset.Ico; I think that it's probably best if I do this properly and add them. I'm still working on this PR so I'll leave it as awaiting-author.

archive/imo/imo1979_q1.lean Outdated Show resolved Hide resolved
archive/imo/imo1979_q1.lean Outdated Show resolved Hide resolved
archive/imo/imo1979_q1.lean Outdated Show resolved Hide resolved
archive/imo/imo1979_q1.lean Outdated Show resolved Hide resolved
archive/imo/imo1979_q1.lean Outdated Show resolved Hide resolved
archive/imo/imo1979_q1.lean Outdated Show resolved Hide resolved
archive/imo/imo1979_q1.lean Outdated Show resolved Hide resolved
archive/imo/imo1979_q1.lean Outdated Show resolved Hide resolved
@kbuzzard kbuzzard added the WIP Work in progress label Nov 18, 2020
archive/imo/imo1979_q1.lean Outdated Show resolved Hide resolved
@[reducible, nolint fails_quickly] def e : ℚ := ∑ n in range 330, 1 / (n + 660)
@[reducible, nolint fails_quickly] def f : ℚ := ∑ n in range 330, 1 / (1319 - n)

/-
Copy link
Member

@jcommelin jcommelin Nov 19, 2020

Choose a reason for hiding this comment

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

Is this a module doc?

Suggested change
/-
/-!

@github-actions github-actions bot added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Nov 21, 2020
@kbuzzard kbuzzard removed the awaiting-author A reviewer has asked the author a question or requested changes label Nov 22, 2020
@kbuzzard
Copy link
Member Author

Just to clarify what's going on here -- I wanted the parity stuff for another reason so I PR'ed it independently; I embarked upon adding some finset.Ico stuff so I could streamline the sum proofs, but then I heard here that @Vierkantor was in the middle of refactoring it all on the fin_range branch so I'm leaving this for now. I'm happy to close this or wait until fin_range is PR'ed. In the mean time I might PR the non-IMO part of this PR separately.

@github-actions github-actions bot removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Nov 22, 2020
@YaelDillies
Copy link
Collaborator

Now that all the finset.Ixx stuff has been generalized, this can go forward.

@YaelDillies YaelDillies reopened this Oct 19, 2021
@github-actions github-actions bot added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Oct 19, 2021
@github-actions github-actions bot removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Oct 19, 2021
@github-actions github-actions bot added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Oct 26, 2021
@github-actions github-actions bot removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Oct 26, 2021
@MantasBaksys
Copy link
Collaborator

I have pushed a new commit, where I've updated the file to use finset.Icc, updated the proof to compile, golfed it slightly and deleted definitions, which I and @YaelDillies thought were unnecessary.

@MantasBaksys MantasBaksys added awaiting-review The author would like community review of the PR and removed WIP Work in progress labels Nov 3, 2021

The p-adic valuation function on ℚ is ℤ-valued and hence has v(0)=0 (rather than +∞);
we thus had to occasionally deal with funny edge cases which aren't there mathematically.
In retrospect it might have been easier to work with p-adic norms, which are ℚ-valued
Copy link
Member

Choose a reason for hiding this comment

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

How hard would it be to do this refactor? I think these IMO files should be showcases that look as nice as possible.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@kbuzzard, what did you exactly mean by that? I can't make sense of the "-valued" bit. Surely you meant "with_top ℤ-valued"? If that's really the case, the refactor is arguable.

Copy link
Member

Choose a reason for hiding this comment

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

I think he meant -valued, although the norms of p-adic numbers are all in the image of coe : ℚ → ℝ.

Copy link
Member Author

Choose a reason for hiding this comment

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

I meant that valuations have problems with v(0) but norms don't, |0|=0. We have |p^n|=p^{-n} so \Q will do.

Comment on lines +63 to +64
/-- The injection ℕ ↪ ℕ sending any n ∈ ℕ to 2 * n
-/
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/-- The injection ℕ ↪ ℕ sending any n ∈ ℕ to 2 * n
-/
/-- The injection `ℕ ↪ ℕ` sending any `n ∈ ℕ` to `2 * n` -/

@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 Nov 4, 2021
@urkud
Copy link
Member

urkud commented Jul 17, 2022

@kbuzzard Could you please move this PR to https://github.com/leanprover-community/mathzoo ?

@kbuzzard
Copy link
Member Author

@YaelDillies are you still thinking about working on this?

@kbuzzard
Copy link
Member Author

I'm merging master and I find that there is some missing API for padic_norm. I've added some in #16074 . @urkud The problem with putting the lemma in mathzoo is that API won't be there; the last time they bumped mathlib was March.

@kbuzzard
Copy link
Member Author

kbuzzard commented Dec 18, 2022

Yael reopened this but I'm going to close it again because I see no point in working on a Lean 3 version of this, with mathlib4 so near.

@kbuzzard kbuzzard closed this Dec 18, 2022
@eric-wieser
Copy link
Member

because I see no point in working on a Lean 3 version of this, with mathlib4 so near.

Mathport can always translate it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-author A reviewer has asked the author a question or requested changes imo Formalisation of an IMO problem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants