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: Nat.shiftRight_bitwise_distrib #6334

Merged
merged 2 commits into from
Dec 11, 2024

Conversation

tydeu
Copy link
Member

@tydeu tydeu commented Dec 8, 2024

This PR adds Nat theorems for distributing >>> over bitwise operations, paralleling those of BitVec.

This enables closing goals like the following using simp:

example (n : Nat) : (n <<< 2 ||| 3) >>> 2 = n := by simp [Nat.shiftRight_or_distrib]

It might be nice for these theorems to be simp lemmas, but they are not currently in order to be consistent with the existing BitVec and div_two theorems.

@tydeu tydeu added the changelog-library Library label Dec 8, 2024
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Dec 8, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Dec 8, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Dec 8, 2024
@tydeu tydeu marked this pull request as ready for review December 8, 2024 04:37
@tydeu tydeu requested a review from kim-em as a code owner December 8, 2024 04:37
@tydeu tydeu marked this pull request as draft December 8, 2024 04:38
@tydeu tydeu marked this pull request as ready for review December 8, 2024 04:38
@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Dec 8, 2024

Mathlib CI status (docs):

@tobiasgrosser
Copy link
Contributor

It might be nice for these theorems to be simp lemmas, but they are not currently in order to be consistent with the existing BitVec and div_two theorems.

Can you expand on this? This PR adds:

theorem shiftRight_and_distrib {a b : Nat} : (a &&& b) >>> i = a >>> i &&& b >>> i :=
  shiftRight_bitwise_distrib

while the BitVev library has

 theorem ushiftRight_and_distrib (x y : BitVec w) (n : Nat) :
    (x &&& y) >>> n = (x >>> n) &&& (y >>> n) := by
  ext
  simp

This seem to be consistent among data types. What would be needed to make these simp lemmas? AFAIU neither direction is strictly more canonical. Do you have specific improvements in mind? I would be happy to explore these in the BitVec library.

@tydeu
Copy link
Member Author

tydeu commented Dec 9, 2024

@tobiasgrosser

What would be needed to make these simp lemmas? AFAIU neither direction is strictly more canonical. Do you have specific improvements in mind? I would be happy to explore these in the BitVec library.

My thesis is that distributing shiftRight (and factoring out shiftLeft) is the natural direction because both reduce the number of bits considered by the bitwise operations. This reduces complexity and increases the chance the rest can be simp'ed away. I think the example in the PR description is a good demonstration of the kind of expression that it would be great to be able to simp by default:

example (n : Nat) : (n <<< 2 ||| 3) >>> 2 = n := by simp

Nonetheless, there are cases were distributing shiftRight can be bad. If the subterm can already simplified due to the presence of literals or a hypothesis, distributing can impair this. For instance:

example (n : Nat) : (1 &&& 3) >>> n = 1 >>> n := by simp 

Distributing first here would create the term (1 >>> n &&& 3 >>> n) which can no longer be simp'ed. Luckily, this is not a problem if distribution is a bottom-up simplification lemma (i.e., @[simp ↑], the default), as simp will apply potential simplifications to the subterm before distributing (and then simplifying again).

Weighing thees pros and cons lead me to believe the making the shiftRight distribution lemmas @[simp] would be preferable. However, I am not an expert in this area and there could be cases I have not considered where this poses a problem, hence my initial reservation.

@kim-em kim-em self-assigned this Dec 10, 2024
@kim-em
Copy link
Collaborator

kim-em commented Dec 10, 2024

Otherwise, LGTM.

@kim-em kim-em removed their assignment Dec 10, 2024
@tydeu tydeu force-pushed the nat-shiftRight-bitwise-distrib branch from 3e14908 to a0af185 Compare December 10, 2024 04:01
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Dec 10, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Dec 10, 2024
@leanprover-community-bot leanprover-community-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Dec 10, 2024
@kim-em kim-em added this pull request to the merge queue Dec 11, 2024
Merged via the queue into leanprover:master with commit a64a17e Dec 11, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builds-mathlib CI has verified that Mathlib builds against this PR changelog-library Library toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants