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/nat/digits): add last_digit_ne_zero #3544

Closed
wants to merge 8 commits into from

Conversation

shingtaklam1324
Copy link
Collaborator

The proof of base_pow_length_digits_le was refactored as suggested by @fpvandoorn in #3498.


@semorrison semorrison added the awaiting-review The author would like community review of the PR label Jul 25, 2020
src/data/nat/digits.lean Outdated Show resolved Hide resolved
src/data/nat/digits.lean Outdated Show resolved Hide resolved
src/data/nat/digits.lean Outdated Show resolved Hide resolved
src/data/nat/digits.lean Outdated Show resolved Hide resolved
src/data/nat/digits.lean Outdated Show resolved Hide resolved
@semorrison semorrison 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 Jul 25, 2020
@shingtaklam1324 shingtaklam1324 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 Jul 25, 2020
@fpvandoorn fpvandoorn 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 Jul 25, 2020
Copy link
Member

@fpvandoorn fpvandoorn left a comment

Choose a reason for hiding this comment

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

The proof of the final result looks much better now! Thanks for making these changes.

src/data/nat/digits.lean Outdated Show resolved Hide resolved
src/data/nat/digits.lean Outdated Show resolved Hide resolved
src/data/nat/digits.lean Outdated Show resolved Hide resolved
src/data/nat/digits.lean Outdated Show resolved Hide resolved
src/data/nat/digits.lean Outdated Show resolved Hide resolved
end

lemma last_digit_ne_zero (b m : ℕ) (hm : m ≠ 0) (p):
(digits b m).last p ≠ 0 :=
Copy link
Member

Choose a reason for hiding this comment

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

This lemma would be more convenient if you remove the argument p and explicitly prove it using digits_ne_nil_iff_ne_zero.mpr hm.

Copy link
Collaborator Author

@shingtaklam1324 shingtaklam1324 Jul 25, 2020

Choose a reason for hiding this comment

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

To be honest that was my first approach to use digits_ne_nil_iff_ne_zero.mpr hm. I really struggled to get nat.strong_induction_on to work with it, hence I've generalized over the proof. (see the use of revert p in the proof). There might be a better approach but this is the best I could come up with.

I can mark this as private and add in a lemma with the proof provided, would that be ok? So change this to private lemma last_digit_ne_zero_aux and add

lemma last_digit_ne_zero {b m : ℕ} (hm : m ≠ 0) :
  (digits b m).last (digits_ne_nil_iff_ne_zero.mpr hm) ≠ 0 :=
last_digit_ne_zero_aux b m hm (digits_ne_nil_iff_ne_zero.mpr hm)

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 done the above. So marked this one as private lemma last_digit_ne_zero_aux and then added another lemma that fills in the proof.

Copy link
Member

Choose a reason for hiding this comment

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

That works. Another option was to use as first tactic generalize : digits_ne_nil_iff_ne_zero.mpr hm = p,

exact nat.div_pos (le_of_not_lt hnb) dec_trivial } },
end

lemma last_digit_ne_zero (b : ℕ) {m : ℕ} (hm : m ≠ 0) :
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For this, I think m can be figured out by later hypotheses, but b can't be. Right?

Although in the only use of this lemma so far, b could be inferred.

Copy link
Member

Choose a reason for hiding this comment

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

That is correct. It depends a bit on the lemma whether we make the argument b explicit in this case (most of the time, but not always, b can be figured out from the statement you're proving at that time).

@shingtaklam1324 shingtaklam1324 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 Jul 26, 2020
@fpvandoorn
Copy link
Member

bors merge

@github-actions github-actions bot 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 Jul 27, 2020
bors bot pushed a commit that referenced this pull request Jul 27, 2020
The proof of `base_pow_length_digits_le` was refactored as suggested by @fpvandoorn in #3498.
@bors
Copy link

bors bot commented Jul 27, 2020

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(data/nat/digits): add last_digit_ne_zero [Merged by Bors] - feat(data/nat/digits): add last_digit_ne_zero Jul 27, 2020
@bors bors bot closed this Jul 27, 2020
@bors bors bot deleted the last-digit branch July 27, 2020 01:17
bors bot pushed a commit that referenced this pull request Aug 17, 2020
I thoroughly misunderstood why my prior attempts for #3544 weren't working. I've refactored the proof so the `private` lemma is no longer necessary.
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

4 participants