Skip to content

Refactor Data.Vec.Properties : Add toList-injective and new lemmas #2733

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

jmougeot
Copy link
Contributor

@jmougeot jmougeot commented Jun 9, 2025

  • Replace Data.List.Base as List and Data.List.Properties as List with shorter L aliases throughout the module for better readable proof

  • Add new utility lemma toList-injective that proves vector equality from list equality using heterogeneous equality

  • Add new equation-free lemmas that leverage toList-injective for cleaner proofs:

    • fromList-reverse: proves fromList (L.reverse xs) ≈[ L.length-reverse xs ] reverse (fromList xs)
    • ++-ʳ++-eqFree: proves associativity-like property for reverse-append without explicit equation parameter
    • ʳ++-ʳ++-eqFree: proves composition property for reverse-append operations
  • Update all List. references to L. in function types and implementations

  • Import Tactic.Cong module to support the cong! tactic used in several proofs

This change improves code readability and adds a fundamental lemma for reasoning about vector equality via list conversion, which is used in several proofs throughout the module.

@JacquesCarette
Copy link
Contributor

Replace Data.List.Base as List and Data.List.Properties as List with shorter L aliases throughout the module for better readable proof

While I happen to like that, this goes specifically against the stdlib naming convention which states that it should be List.

import Tactic.Cong module to support the cong! tactic used in several proofs

It has been an (unwritten!) convention to not use tactics in fairly low-level parts of the library, as we don't want those parts of the library to depend on the Reflection machinery. The dependency graph is nasty enough as it is, we don't really want to make it worse.

Copy link
Contributor

@MatthewDaggitt MatthewDaggitt left a comment

Choose a reason for hiding this comment

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

Generally the lemmas look good, but I agree with @JacquesCarette point about the use of tactics.

→ reverse (xs ++ ys) ≈[ +-comm m n ] reverse ys ++ reverse xs
reverse-++-eqFree {m = m} {n = n} xs ys =
toList-injective (+-comm m n) (reverse (xs ++ ys)) (reverse ys ++ reverse xs) $
begin
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really a cleaner proof? To my eyes it looks longer and more complicated?

fromList-reverse : (xs : List A) → (fromList (List.reverse xs)) ≈[ List.length-reverse xs ] reverse (fromList xs)
fromList-reverse xs = toList-injective (List.length-reverse xs) (fromList (List.reverse xs)) (reverse (fromList xs)) $
begin
toList (fromList (List.reverse xs)) ≡⟨ toList∘fromList (List.reverse xs) ⟩
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we align the reasoning combinators?

@@ -1304,8 +1339,8 @@ cast-fromList {xs = List.[]} {ys = List.[]} eq = refl
cast-fromList {xs = x List.∷ xs} {ys = y List.∷ ys} eq =
let x≡y , xs≡ys = List.∷-injective eq in begin
x ∷ cast (cong (pred ∘ List.length) eq) (fromList xs) ≡⟨ cong (_ ∷_) (cast-fromList xs≡ys) ⟩
x ∷ fromList ys ≡⟨ cong (_∷ _) x≡y ⟩
y ∷ fromList ys ∎
x ∷ fromList ys ≡⟨ cong (_∷ _) x≡y ⟩
Copy link
Contributor

Choose a reason for hiding this comment

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

The only change here seems to be to misalign the combinators?

@@ -52,6 +53,19 @@ private
m n o : ℕ
ws xs ys zs : Vec A n

------------------------------------------------------------------------
-- Properties to List
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be Properties of toList


toList-injective
: ∀ {m n}
→ .(m=n : m ≡ n)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we follow the style guide about the placement of arrows on new lines?

Also can the name of this proof use instead of =?

-- Properties to List

toList-injective
: ∀ {m n}
Copy link
Contributor

Choose a reason for hiding this comment

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

This are variables and therefore not needed.

toList-reverse : ∀ (xs : Vec A n) → toList (reverse xs) ≡ List.reverse (toList xs)
toList-reverse [] = refl
toList-reverse (x ∷ xs) = begin
toList (reverse (x ∷ xs)) ≡⟨ cong toList (reverse-∷ x xs) ⟩
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we align the reasoning combinators? (you may need to install a monospaced font to make this easier)

where open CastReasoning
toList-ʳ++ : ∀ (xs : Vec A m) {ys : Vec A n} → toList (xs ʳ++ ys) ≡ (toList xs) List.ʳ++ toList ys
toList-ʳ++ xs {ys} = begin
toList (xs ʳ++ ys) ≡⟨ cong toList (unfold-ʳ++ xs ys) ⟩
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we align the reasoning combinators? (for all new proofs in this file)

reverse (map f xs) ++ map f ys ≡⟨ unfold-ʳ++ (map f xs) (map f ys) ⟨
map f xs ʳ++ map f ys ∎
where open ≡-Reasoning

∷-ʳ++-eqFree : ∀ a (xs : Vec A m) {ys : Vec A n} → let eq = sym (+-suc m n) in
cast eq ((a ∷ xs) ʳ++ ys) ≡ xs ʳ++ (a ∷ ys)
∷-ʳ++-eqFree a xs {ys} = begin
Copy link
Contributor

Choose a reason for hiding this comment

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

This lemma seems to have been deleted without replacement?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants