feat: Add Array.scan{l,r,lM,rM}#1589
Conversation
|
WIP |
|
awaiting-review |
| (a.eraseIdxIfInBounds i).size = if i < a.size then a.size-1 else a.size := by | ||
| grind | ||
|
|
||
| theorem toList_drop (as: Array α) (n : Nat) : |
There was a problem hiding this comment.
| theorem toList_drop (as: Array α) (n : Nat) : | |
| @[simp, grind =] | |
| theorem toList_drop (as: Array α) (n : Nat) : |
to match toList_erase above.
There was a problem hiding this comment.
This doesn't work as a simp lemma because simp simplifies '(as.drop n).toList' to 'List.take (as.size - n) (List.drop n as.toList)`
There was a problem hiding this comment.
Fwiw I think this is very frustrating. Array.drop is automatically simplified to Array.extract. This over-eager simplification is why I wanted a toList_drop method to begin with.
There was a problem hiding this comment.
There are way too many simp lemmas for my taste. Eric is very fond of them though and I respect his opinion. We (in general, including Eric and I) need a common language to talk about simp-normal-form. The Mathlib/Batteries concerns are a bit different.
There was a problem hiding this comment.
Thanks for starting the Zulip thread; indeed this can't be simp right now.
There was a problem hiding this comment.
Can we resolve this or are we hoping to fix the simp issues upstream?
| theorem toArray_scanl {f : β → α → β} {as : List α} : | ||
| (as.scanl f init).toArray = as.toArray.scanl f init := by | ||
| rw [← Array.toList_scanl] | ||
|
|
||
| theorem toArray_scanr {f : α → β → β} {as : List α} : | ||
| (as.scanr f init).toArray = as.toArray.scanr f init := by | ||
| rw [← Array.toList_scanr] |
There was a problem hiding this comment.
I was going to claim that these should be simp, but having found #lean4 > Direction of List.map_toArray vs Array.toList_map @ 💬 I'm no longer sure.
There was a problem hiding this comment.
Can we proceed without these simps?
|
Our Mathlib Testing CI is partly broken at this time. The last run has reported an error: https://github.com/leanprover-community/mathlib4-nightly-testing/tree/batteries-pr-testing-1589 Could you investigate to see whether there needs to be a Mathlib adaptation or whether this is a false negative? |
|
leanprover-community/mathlib4#35228 I opened a PR that fixes the issue. The problem was that I added List.reverse_singleton, but that was something that already existed in mathlib. The simple solution is to replace it. |
|
Thanks! This is not the preferred way to make a Mathlib adaptation PR but I think we can work with it. The preferred way is to edit the Mathlib Testing branch until CI works and then make a PR from that branch into Mathlib. I've added your fix to that branch and we will see what CI says, in case we need to add some additional fixes. If CI clears, then we can just use your Mathlib PR, otherwise we will revert to the preferred route. @kim-em Could you give @cmlsharp write access to Mathlib Testing. (Is there a way to automate this?) |
|
Oh sorry about that! I will do that in the future. I just realized there was a commit I meant to push prior to making the PR (marking this lemma simp since it is marked that way in mathlib). I just pushed it. |
After [batteries#1589](leanprover-community/batteries#1589) is merged: - [x] Merge leanprover-community/mathlib4:master - [x] Edit the lakefile to point to leanprover-community/batteries:main - [x] Run lake update batteries - [ ] Wait for CI and merge Co-authored-by: leanprover-community-mathlib4-bot <leanprover-community-mathlib4-bot@users.noreply.github.com> Co-authored-by: F. G. Dorais <fgdorais@gmail.com>
After [batteries#1589](leanprover-community/batteries#1589) is merged: - [x] Merge leanprover-community/mathlib4:master - [x] Edit the lakefile to point to leanprover-community/batteries:main - [x] Run lake update batteries - [ ] Wait for CI and merge Co-authored-by: leanprover-community-mathlib4-bot <leanprover-community-mathlib4-bot@users.noreply.github.com> Co-authored-by: F. G. Dorais <fgdorais@gmail.com>
After [batteries#1589](leanprover-community/batteries#1589) is merged: - [x] Merge leanprover-community/mathlib4:master - [x] Edit the lakefile to point to leanprover-community/batteries:main - [x] Run lake update batteries - [ ] Wait for CI and merge Co-authored-by: leanprover-community-mathlib4-bot <leanprover-community-mathlib4-bot@users.noreply.github.com> Co-authored-by: F. G. Dorais <fgdorais@gmail.com>
Co-authored-by: François G. Dorais <fgdorais@gmail.com> Co-authored-by: Kim Morrison <477956+kim-em@users.noreply.github.com> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: Gavin Zhao <gavinzhaojw@protonmail.com> Co-authored-by: Ruben Van de Velde <65514131+Ruben-VandeVelde@users.noreply.github.com> Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
After [batteries#1589](leanprover-community/batteries#1589) is merged: - [x] Merge leanprover-community/mathlib4:master - [x] Edit the lakefile to point to leanprover-community/batteries:main - [x] Run lake update batteries - [ ] Wait for CI and merge Co-authored-by: leanprover-community-mathlib4-bot <leanprover-community-mathlib4-bot@users.noreply.github.com> Co-authored-by: F. G. Dorais <fgdorais@gmail.com>
This PR adds the Array equivalents of List.scan{l,r,L,R}. It was originally part of #1581.
Array.scanlandArray.scanrin terms of them. This is identical to how the standard library currently handlesArray.fold*.unsafein the implementations of the efficient versions. It defines an assumption (asArray.size_fits_usize) as well as anunsafeproof of this assumption (asArray.unsafe_size_fits_usize).These are currently public definitions. I could make them private, but they seem useful for writing other 'efficient' array functions.Array.scanlMis proven to be equivalent to its efficient implementation under the assumption thatArray.size_fits_usizeholds. The efficient version ofArray.scanrMadditionally uses this assumption when constructing the proofs for the bounds checks.Array.scanrMis equivalent to its efficient version underArray.size_fits_usize. It should be possible, but the structure of these two implementations differ more, so the proof is more difficult. WithArray.scanlMthe only difference between the implementations was the use ofUSizeorNatfor the index, whereas theArray.scanrMreference implementation builds the array, then reverses it and the efficient implementation iterates over the array in reverse.