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: Define dependent version of Fin.foldl #1071

Merged
merged 15 commits into from
Dec 4, 2024

Conversation

quangvdao
Copy link
Contributor

@quangvdao quangvdao commented Dec 2, 2024

This PR defines the dependent versions of Fin.fold{l/r}{M}. This is 4 functions in total, Fin.dfold{l/r}{M}, and we give similar theorems as the non-dependent versions, plus theorems that relate the dependent to the non-dependent version.

Two theorems are currently missing: dfold{l/r}_rev. Even stating these theorems is a pain due to casting, so I'd prefer to add them later.

@quangvdao quangvdao marked this pull request as draft December 2, 2024 01:48
Batteries/Data/Fin/Fold.lean Outdated Show resolved Hide resolved
Batteries/Data/Fin/Fold.lean Outdated Show resolved Hide resolved
@kim-em
Copy link
Collaborator

kim-em commented Dec 2, 2024

Can you write the theorem relating it to the usual Fin.fold, with \alpha := \Sigma i, \alpha i?

@kim-em
Copy link
Collaborator

kim-em commented Dec 2, 2024

Other directions that would be good:

  • just as we have Fin.fold and Nat.fold and a theorem relating them, we should have the "unbundled" version of this for Nat, and the theorem
  • we should have the monadic version as well.

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict This PR has merge conflicts with the `main` branch which must be resolved by the author. label Dec 2, 2024
@quangvdao
Copy link
Contributor Author

Can you write the theorem relating it to the usual Fin.fold, with \alpha := \Sigma i, \alpha i?

Actually, I don't know how to state the theorem. A natural attempt is:

theorem fold_eq_foldl_sigma {α : Fin (n+1) → Sort _} (f : ∀ (i : Fin n), α i.castSucc → α i.succ)
    (x : α 0) :
    fold n α f x = foldl n (α := (i : Fin (n + 1)) × α i) (fun x i => f x.1 x.2) ⟨0, x⟩ := sorry

but we run into the problem of Lean not knowing that x.1 is supposed to be the same as i.castSucc during execution.

@quangvdao
Copy link
Contributor Author

quangvdao commented Dec 2, 2024

  • just as we have Fin.fold and Nat.fold and a theorem relating them, we should have the "unbundled" version of this for Nat, and the theorem
  • I agree, though not quite sure where to put things (Data/Nat/Basic and Data/Nat/Lemmas, or a new Data/Nat/Fold).
  • The dependent version of Nat.fold is Fin.hIterate.
  • I can't find the lemma relating the current Fin.foldl with Nat.fold.
  • we should have the monadic version as well.

This has been added.

As written, these new dependent versions add no overhead after compilation (some initial experiments also confirm this). Because of this, do you think we could redefine (say) Fin.foldl as the dependent version of Fin.fold?

Also, in terms of naming, I think writing Fin.fold and Fin.foldRev will align with the Nat version. And I'd also want to change the function signature in Fin.foldl:

Fin.foldl.{u_1} {α : Sort u_1} (n : ℕ) (f : α → Fin n → α) (init : α) : α

to

Fin.foldl.{u_1} {α : Sort u_1} (n : ℕ) (f : Fin n → α → α) (init : α) : α

in order to match the dependent version.

In summary, we can have Fin.fold' as the dependent version, Fin.fold as non-dependent, defined in terms of the former, the foldRev versions, and the monadic versions.

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the merge-conflict This PR has merge conflicts with the `main` branch which must be resolved by the author. label Dec 2, 2024
Copy link
Collaborator

@fgdorais fgdorais left a comment

Choose a reason for hiding this comment

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

foldl and foldr are standard names in functional programming, we should stick to that convention. Use dfoldl/dfoldr or foldlDep/foldrDep.

Batteries/Data/Fin/Basic.lean Outdated Show resolved Hide resolved
Batteries/Data/Fin/Basic.lean Outdated Show resolved Hide resolved
Batteries/Data/Fin/Basic.lean Outdated Show resolved Hide resolved
Batteries/Data/Fin/Basic.lean Outdated Show resolved Hide resolved
Batteries/Data/Fin/Basic.lean Outdated Show resolved Hide resolved
Batteries/Data/Fin/Lemmas.lean Outdated Show resolved Hide resolved
@quangvdao quangvdao marked this pull request as ready for review December 2, 2024 21:57
@fgdorais fgdorais added the awaiting-review This PR is ready for review; the author thinks it is ready to be merged. label Dec 2, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Dec 2, 2024
@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Dec 2, 2024

Mathlib CI status (docs):

leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Dec 3, 2024
@fgdorais
Copy link
Collaborator

fgdorais commented Dec 3, 2024

@kim-em could you protect Fin.cast in Lean 4 when you have time? The few _root_.cast uses here are annoying.

@fgdorais
Copy link
Collaborator

fgdorais commented Dec 3, 2024

Looks good. Make sure to add specialize on the inner loops. If you have time, expand the docstrings using Init.Data.Fin.Fold as a model. I'll have another pass tomorrow after a good night of sleep!

@fgdorais fgdorais added awaiting-author Waiting for PR author to address issues and removed awaiting-review This PR is ready for review; the author thinks it is ready to be merged. labels Dec 3, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Dec 3, 2024
@fgdorais
Copy link
Collaborator

fgdorais commented Dec 3, 2024

Avoid code duplication by defininig:

@[inline] def dfoldrM [Monad m] (n : Nat) (α : Fin (n + 1) → Type _)
    (f : ∀ (i : Fin n), α i.succ → m (α i.castSucc)) (init : α (last n)) : m (α 0) := 
  dfoldr n (fun i => m (α i)) (fun i x => x >>= f i) (pure init)

I'm not sure the same works for dfoldlM.

@quangvdao
Copy link
Contributor Author

Avoid code duplication by defininig:

@[inline] def dfoldrM [Monad m] (n : Nat) (α : Fin (n + 1) → Type _)
    (f : ∀ (i : Fin n), α i.succ → m (α i.castSucc)) (init : α (last n)) : m (α 0) := 
  dfoldr n (fun i => m (α i)) (fun i x => x >>= f i) (pure init)

I'm not sure the same works for dfoldlM.

When I tried to do this, the theorems can't seem to infer the type of α and x.

@fgdorais
Copy link
Collaborator

fgdorais commented Dec 4, 2024

The loop lemmas are no longer relevant after this change. The other lemmas are immediate consequences of the dfoldr lemmas.

Batteries/Data/Fin/Fold.lean Outdated Show resolved Hide resolved
Batteries/Data/Fin/Fold.lean Outdated Show resolved Hide resolved
Batteries/Data/Fin/Fold.lean Outdated Show resolved Hide resolved
Batteries/Data/Fin/Fold.lean Outdated Show resolved Hide resolved
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Dec 4, 2024
@fgdorais
Copy link
Collaborator

fgdorais commented Dec 4, 2024

I completed the last few changes for you. Let me know if you would like to revert something.

leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Dec 4, 2024
@fgdorais
Copy link
Collaborator

fgdorais commented Dec 4, 2024

I just approved but I then noticed that the docstrings should not just refer to other functions. Please expand them and mention the relation with non-dependent functions as an additional note.

@quangvdao
Copy link
Contributor Author

I filled out the docstrings. Thanks for all the help!

leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Dec 4, 2024
@fgdorais fgdorais added this pull request to the merge queue Dec 4, 2024
Merged via the queue into leanprover-community:main with commit c016aa9 Dec 4, 2024
2 checks passed
@kim-em
Copy link
Collaborator

kim-em commented Dec 5, 2024

@kim-em could you protect Fin.cast in Lean 4 when you have time? The few _root_.cast uses here are annoying.

Done in leanprover/lean4#6315

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-author Waiting for PR author to address issues builds-mathlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants