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(data/fin): add some lemmas about coercions #2522

Closed
wants to merge 12 commits into from

Conversation

jsm28
Copy link
Collaborator

@jsm28 jsm28 commented Apr 25, 2020

Two of these lemmas allow norm_cast to work with inequalities
involving fin values converted to ℕ. The rest are for simplifying
expressions where coercions are used to convert from ℕ to fin, in
cases where an inequality means those coercions do not in fact change
the value.

There are very few lemmas relating to coercions from ℕ to fin in
mathlib at present; the lemma of_nat_eq_coe (and val_add on which it
depends, and a few similarly trivial lemmas alongside val_add) is
moved from data.zmod.basic to fin.basic for use in proving the other
lemmas, while the nat lemma add_mod is moved to data.nat.basic for use
in the proof of of_nat_eq_coe, and mul_mod is moved alongside it as
suggested in review. These lemmas were found useful in formalising
solutions to an olympiad problem, see
https://leanprover.zulipchat.com/#narrow/stream/113489-new-members/topic/Some.20olympiad.20formalisations,
and seem more generally relevant than to just that particular problem.

TO CONTRIBUTORS:

Please include a summary of the changes made in this PR above "TO CONTRIBUTORS:", as that text will become the commit message. You are also encouraged to append the following co-authorship template if you'd like to acknowledge suggestions / commits made by other users:

Co-authored-by: name name@example.com

Make sure you have:

If this PR is related to a discussion on Zulip, please include a link in the discussion.

For reviewers: code review check list

If you're confused by comments on your PR like bors r+ or bors d+, please see our notes on bors for information on our merging workflow.

Two of these lemmas allow norm_cast to work with inequalities
involving fin values converted to ℕ.  The rest are for simplifying
expressions where of_nat is used to convert from ℕ to fin, in cases
where an inequality means of_nat does not in fact change the value.

There are very few lemmas relating to of_nat in mathlib at present.
These lemmas were found useful in formalising solutions to an olympiad
problem, see
<https://leanprover.zulipchat.com/#narrow/stream/113489-new-members/topic/Some.20olympiad.20formalisations>,
and seem more generally relevant than to just that particular problem.
@bryangingechen
Copy link
Collaborator

Is there a reason to use nat.succ n instead of n+1? I noticed that the rest of the file uses the latter.

@jsm28
Copy link
Collaborator Author

jsm28 commented Apr 25, 2020

Only that the definition of of_nat (in the core Lean library) uses it. All my uses of these lemmas work unchanged with n+1 used in place of nat.succ n, if this is a matter of the core library using a style that's not preferred for this mathlib code and n+1 is preferred in mathlib.

@bryangingechen
Copy link
Collaborator

It may not matter a lot in practice, since n.succ and n+1 are definitionally equal, thus generally Lean is able to automatically convert between the two as needed. I think we try to keep things consistent for those odd cases where it can't, and I believe we've decided to prefer n+1 in mathlib. See e.g. #2067 (comment).

@jsm28
Copy link
Collaborator Author

jsm28 commented Apr 25, 2020

Now changed to use n + 1.

@bryangingechen
Copy link
Collaborator

The PR looks good to me. I think some of these should be made @[simp], but I'm not 100% sure, so I'll let someone else comment on that.

Thanks for this and the previous contribution! I'll send you an invite so that you can create new branches in this repo and make PRs from them. This is preferred since that way github will display status icons that tell us at a glance whether your PRs build + lint successfully.

Copy link
Member

@gebner gebner left a comment

Choose a reason for hiding this comment

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

I'm not 100% sure these are correct simp lemmas. Write #lint at the end of the file, it will tell you if these should not be marked as simp lemmas.

src/data/fin.lean Outdated Show resolved Hide resolved
src/data/fin.lean Outdated Show resolved Hide resolved
src/data/fin.lean Outdated Show resolved Hide resolved
src/data/fin.lean Outdated Show resolved Hide resolved
src/data/fin.lean Outdated Show resolved Hide resolved
jsm28 and others added 5 commits April 25, 2020 19:42
Co-Authored-By: Gabriel Ebner <gebner@gebner.org>
Co-Authored-By: Gabriel Ebner <gebner@gebner.org>
Co-Authored-By: Gabriel Ebner <gebner@gebner.org>
Co-Authored-By: Gabriel Ebner <gebner@gebner.org>
Co-Authored-By: Gabriel Ebner <gebner@gebner.org>
@jsm28
Copy link
Collaborator Author

jsm28 commented Apr 25, 2020

Those simp lemmas all passed #lint, so applied the suggested changes.

src/data/fin.lean Outdated Show resolved Hide resolved
@urkud
Copy link
Member

urkud commented Apr 25, 2020

bors merge

@urkud urkud added the ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) label Apr 25, 2020
bors bot pushed a commit that referenced this pull request Apr 26, 2020
Two of these lemmas allow norm_cast to work with inequalities
involving fin values converted to ℕ.  The rest are for simplifying
expressions where of_nat is used to convert from ℕ to fin, in cases
where an inequality means of_nat does not in fact change the value.

There are very few lemmas relating to of_nat in mathlib at present.
These lemmas were found useful in formalising solutions to an olympiad
problem, see
<https://leanprover.zulipchat.com/#narrow/stream/113489-new-members/topic/Some.20olympiad.20formalisations>,
and seem more generally relevant than to just that particular problem.
@bors
Copy link

bors bot commented Apr 26, 2020

Build failed (retrying...):

@bryangingechen
Copy link
Collaborator

The build failed on linting some of the simp lemmas: https://github.com/leanprover-community/mathlib/runs/618907915#step:14:68

bors r-

@bors
Copy link

bors bot commented Apr 26, 2020

Canceled.

@bryangingechen bryangingechen added awaiting-author A reviewer has asked the author a question or requested changes and removed ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) labels Apr 26, 2020
@jsm28 jsm28 changed the title feat(data/fin): add some lemmas about coercions and of_nat feat(data/fin): add some lemmas about coercions Apr 26, 2020
@jsm28
Copy link
Collaborator Author

jsm28 commented Apr 26, 2020

I interpret that lint failure as indicating that the presence of of_nat_eq_coe (in data.zmod.basic) means all these lemmas should actually be expressed in terms of coercions from ℕ to fin, and in general any proofs working with such conversions should prefer to express them in terms of those coercions rather than in terms of of_nat. Thus I've rephrased all the new lemmas (not just the simp ones) accordingly, removing the of_nat_one lemma and no longer marking of_nat_zero as a simp lemma. Proving some of the lemmas in their new form uses of_nat_eq_coe. To avoid circular imports, data.fin cannot import data.zmod.basic, so of_nat_eq_coe needed to move to data.fin (which seems to me to be a better place for it anyway); the proof uses val_add, which is also moved, and nat.add_mod, which needs to move from data.nat.modeq to data.nat.basic (thus getting a different proof that's usable in that context) to be usable from data.fin.

@bryangingechen bryangingechen added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Apr 26, 2020
@bryangingechen
Copy link
Collaborator

bors try

bors bot added a commit that referenced this pull request Apr 26, 2020
@bors
Copy link

bors bot commented Apr 26, 2020

try

Build succeeded:

src/data/zmod/basic.lean Outdated Show resolved Hide resolved
@@ -73,9 +73,6 @@ def comm_semigroup (n : ℕ) : comm_semigroup (fin (n+1)) :=

local attribute [instance] fin.add_comm_semigroup fin.comm_semigroup

lemma val_add {n : ℕ} : ∀ a b : fin n, (a + b).val = (a.val + b.val) % n
| ⟨_, _⟩ ⟨_, _⟩ := rfl

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should some of the other fin-related lemmas be moved to data.fin as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've moved the trivial lemmas val_mul, one_val and zero_val that seem reasonably to go along with val_add. The rest look more specific to how fin is being used to set up zmod n.

src/data/nat/modeq.lean Show resolved Hide resolved
Copy link
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

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

Thanks for these PRs! It's much appreciated that you try to fill in these holes. It seems like you jumped into a messy import tangle... I'm sorry. But overall this PR looks good to me. We could try to split some of the longer .basic files, so that we can put things in the logical place without creating circular deps. But that would be work for a follow-up PR, so don't worry about it now.

src/data/nat/basic.lean Outdated Show resolved Hide resolved
src/data/nat/modeq.lean Show resolved Hide resolved
jsm28 and others added 2 commits April 28, 2020 11:34
Co-Authored-By: Johan Commelin <johan@commelin.net>
Copy link
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

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

LGTM

bors merge

bors bot pushed a commit that referenced this pull request Apr 28, 2020
Two of these lemmas allow norm_cast to work with inequalities
involving fin values converted to ℕ.  The rest are for simplifying
expressions where coercions are used to convert from ℕ to fin, in
cases where an inequality means those coercions do not in fact change
the value.

There are very few lemmas relating to coercions from ℕ to fin in
mathlib at present; the lemma of_nat_eq_coe (and val_add on which it
depends, and a few similarly trivial lemmas alongside val_add) is
moved from data.zmod.basic to fin.basic for use in proving the other
lemmas, while the nat lemma add_mod is moved to data.nat.basic for use
in the proof of of_nat_eq_coe, and mul_mod is moved alongside it as
suggested in review.  These lemmas were found useful in formalising
solutions to an olympiad problem, see
<https://leanprover.zulipchat.com/#narrow/stream/113489-new-members/topic/Some.20olympiad.20formalisations>,
and seem more generally relevant than to just that particular problem.
@bryangingechen bryangingechen added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Apr 28, 2020
@bors
Copy link

bors bot commented Apr 28, 2020

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(data/fin): add some lemmas about coercions [Merged by Bors] - feat(data/fin): add some lemmas about coercions Apr 28, 2020
@bors bors bot closed this Apr 28, 2020
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 15, 2020
…#2522)

Two of these lemmas allow norm_cast to work with inequalities
involving fin values converted to ℕ.  The rest are for simplifying
expressions where coercions are used to convert from ℕ to fin, in
cases where an inequality means those coercions do not in fact change
the value.

There are very few lemmas relating to coercions from ℕ to fin in
mathlib at present; the lemma of_nat_eq_coe (and val_add on which it
depends, and a few similarly trivial lemmas alongside val_add) is
moved from data.zmod.basic to fin.basic for use in proving the other
lemmas, while the nat lemma add_mod is moved to data.nat.basic for use
in the proof of of_nat_eq_coe, and mul_mod is moved alongside it as
suggested in review.  These lemmas were found useful in formalising
solutions to an olympiad problem, see
<https://leanprover.zulipchat.com/#narrow/stream/113489-new-members/topic/Some.20olympiad.20formalisations>,
and seem more generally relevant than to just that particular problem.
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 16, 2020
…#2522)

Two of these lemmas allow norm_cast to work with inequalities
involving fin values converted to ℕ.  The rest are for simplifying
expressions where coercions are used to convert from ℕ to fin, in
cases where an inequality means those coercions do not in fact change
the value.

There are very few lemmas relating to coercions from ℕ to fin in
mathlib at present; the lemma of_nat_eq_coe (and val_add on which it
depends, and a few similarly trivial lemmas alongside val_add) is
moved from data.zmod.basic to fin.basic for use in proving the other
lemmas, while the nat lemma add_mod is moved to data.nat.basic for use
in the proof of of_nat_eq_coe, and mul_mod is moved alongside it as
suggested in review.  These lemmas were found useful in formalising
solutions to an olympiad problem, see
<https://leanprover.zulipchat.com/#narrow/stream/113489-new-members/topic/Some.20olympiad.20formalisations>,
and seem more generally relevant than to just that particular problem.
cipher1024 pushed a commit to cipher1024/mathlib that referenced this pull request Mar 15, 2022
…#2522)

Two of these lemmas allow norm_cast to work with inequalities
involving fin values converted to ℕ.  The rest are for simplifying
expressions where coercions are used to convert from ℕ to fin, in
cases where an inequality means those coercions do not in fact change
the value.

There are very few lemmas relating to coercions from ℕ to fin in
mathlib at present; the lemma of_nat_eq_coe (and val_add on which it
depends, and a few similarly trivial lemmas alongside val_add) is
moved from data.zmod.basic to fin.basic for use in proving the other
lemmas, while the nat lemma add_mod is moved to data.nat.basic for use
in the proof of of_nat_eq_coe, and mul_mod is moved alongside it as
suggested in review.  These lemmas were found useful in formalising
solutions to an olympiad problem, see
<https://leanprover.zulipchat.com/#narrow/stream/113489-new-members/topic/Some.20olympiad.20formalisations>,
and seem more generally relevant than to just that particular problem.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants