Skip to content

Conversation

@marcusrossel
Copy link
Contributor

As discussed in leanprover/lean4#8673.

@github-actions github-actions bot added awaiting-review This PR is ready for review; the author thinks it is ready to be merged. and removed awaiting-review This PR is ready for review; the author thinks it is ready to be merged. labels Jun 7, 2025
@fgdorais
Copy link
Collaborator

fgdorais commented Jun 8, 2025

Please summarize the discussion rather than just linking to it. That makes it easier for people here to catch up.

leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 8, 2025

namespace Nat

theorem isDigit_digitChar_iff_lt : n.digitChar.isDigit ↔ (n < 10) :=
Copy link
Member

Choose a reason for hiding this comment

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

Arguably this should be

Suggested change
theorem isDigit_digitChar_iff_lt : n.digitChar.isDigit ↔ (n < 10) :=
@[simp]
theorem isDigit_digitChar : n.digitChar.isDigit = n.blt 10 :=

Copy link
Contributor

Choose a reason for hiding this comment

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

Nat.blt though..? I'd rather do decide (n < 10) in that case

Copy link
Member

Choose a reason for hiding this comment

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

I don't mind much about the RHS, I just assumed that blt was simp-normal

Copy link
Contributor

Choose a reason for hiding this comment

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

Nat.blt is only used in the construction of DecidableLT Nat (and in some other places where fast kernel reduction matters)

Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised that there's not a simp lemma in either direction

@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Jun 8, 2025

Mathlib CI status (docs):

leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 8, 2025
@marcusrossel
Copy link
Contributor Author

marcusrossel commented Jun 8, 2025

I've added the lemma toDigits_of_lt_base, and generalized the title of this PR accordingly.

@marcusrossel marcusrossel changed the title feat: add lemma connecting Nat.toDigits and Char.isDigit feat: lemmas about Nat.toDigits Jun 8, 2025
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 8, 2025
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 9, 2025
@marcusrossel
Copy link
Contributor Author

I've added the lemma toDigits_append_toDigits.

@Rob23oba
Copy link
Contributor

Rob23oba commented Jun 9, 2025

I think there should be some general characterisation lemmas to avoid using Nat.toDigitsCore, e.g.:

theorem toDigits_base_zero (n : Nat) : toDigits 0 n = [n.digitChar]
theorem toDigits_base_one (n : Nat) : toDigits 1 n = List.replicate (n + 1) '0'
theorem toDigits_zero (b : Nat) : toDigits b 0 = ['0']
theorem toDigits_of_lt_base (h : n < b) : toDigits b n = [n.digitChar]
theorem toDigits_of_base_le (h : 1 < b) (h' : b ≤ n) : toDigits b n = toDigits b (n / b) ++ [(n % b).digitChar]

Then some of the others might be able to be proven with these instead of toDigitCore.

leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 9, 2025
@marcusrossel
Copy link
Contributor Author

@Rob23oba Is it desirable to have characterisation lemmas for bases 0 and 1? I feel like this almost encourages relying on these "garbage values".

leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 9, 2025
@Rob23oba
Copy link
Contributor

Rob23oba commented Jun 9, 2025

You're right; but at the same time, we characterize garbage values for division etc. I agree that this is a bit different though, so let's maybe wait with those two until there is consensus on leanprover/lean4#8686.

@fgdorais
Copy link
Collaborator

I'm on the fence about this PR. Several lemmas belong in Char and not Nat, but the main issue is that the functions involved are right on the edge with "implementation details".

I do think it's important to have theorems to prove facts about these. I would recommend a different approach:

  1. Add a function that calculates the representation in an arbitrary base b ≥ 2 of a Nat (as a List (Fin b) or an Array (Fin b) or both, whichever is most convenient).
  2. Add theorems that prove the basic arithmetic properties of the base b representation for the function from item 1.
  3. Prove the correctness of core's implementation by proving that it is a map of the function from item 1 for relevant bases.

I don't think string representation is too relevant. For example, Batteries will eventually need to support Base-64 encoding, which is incompatible with traditional representation in bases 2, 8, 10, 16.

@eric-wieser
Copy link
Member

  1. Add a function that calculates the representation in an arbitrary base b ≥ 2 of a Nat (as a List (Fin b) or an Array (Fin b) or both, whichever is most convenient).

Isn't this basically the Nat.digits function that's already in mathlib?

@fgdorais
Copy link
Collaborator

Perhaps. If so, that should make it easier to adapt this PR!

@eric-wieser
Copy link
Member

I'm not sure I understand the proposal; surely we don't want a third version of this function?

Is the suggestion to upstream Nat.digits? If so, it probably would save a lot of review time to just copy the file exactly from mathlib, and then make a subsequence cleanup if necessary.

@fgdorais
Copy link
Collaborator

I checked out Nat.digits in Mathlib. It is not suitable for Batteries but what is needed would be a more efficient drop-in replacement for Nat.digitsAux. I'm happy to do that in the next couple of weeks.

@marcusrossel How urgent is your need for this PR?

@marcusrossel
Copy link
Contributor Author

@fgdorais not urgent at all :)
I'd also be happy to help with the version you have in mind.

@fgdorais
Copy link
Collaborator

WIP PR #1293 adds the functions I'm thinking about. The toDigitsBE function is the main one needed here. However, I haven't finished proving its correctness. toDigitsLE .. |>.reverse can be used in the mean time but that function is not as efficient as toDigitsBE.

@marcusrossel If you are still inclined to help, you can start reviewing this PR when you have time. The parts leading up to toDigitsBE are unlikely to change in non-trivial ways before final version.

@marcusrossel
Copy link
Contributor Author

I'll be rather busy the next 2 weeks, but if it's not too late by then, I'll take a look :)

@fgdorais fgdorais added this pull request to the merge queue Jul 19, 2025
Merged via the queue into leanprover-community:main with commit 357e60c Jul 19, 2025
1 check passed
@github-actions github-actions bot removed the awaiting-review This PR is ready for review; the author thinks it is ready to be merged. label Jul 19, 2025
@marcusrossel
Copy link
Contributor Author

Uh, @fgdorais was this merge intentional?

@fgdorais
Copy link
Collaborator

Yes! We can improve this later and it doesn't hurt to have it now.

@marcusrossel marcusrossel deleted the to-digits branch July 19, 2025 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants