-
Notifications
You must be signed in to change notification settings - Fork 257
Add new module Effect.Functor.Naperian
- Continuation of #2004
#2815
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
base: master
Are you sure you want to change the base?
Add new module Effect.Functor.Naperian
- Continuation of #2004
#2815
Conversation
with two records : `RawNaperian` and `Naperian`
Added new file `Effect.Functor.Naperian` and two records to it
…tdlib into task2-Naperian
Thanks very much for continuing to work on this, and for responding to the earlier review comments. I refactored the current version, as a way of trying to isolate the details of the contribtion; I'm happy to make local suggestions in a review, but I still think a global change is needed, as follows:
Accordingly: module _ {F : Set a → Set b} c (RF : RawFunctor F) where
-- RawNaperian contains just the functions, not the proofs
record RawNaperian : Set (suc (a ⊔ c) ⊔ b) where
field
Log : Set c
index : F A → (Log → A)
tabulate : (Log → A) → F A
-- Full Naperian has the coherence conditions too.
record Naperian (S : Setoid a ℓ) : Set (suc (a ⊔ c) ⊔ b ⊔ ℓ) where
field
rawNaperian : RawNaperian
open RawNaperian rawNaperian public
open module S = Setoid S
private
FS : Setoid b (c ⊔ ℓ)
FS = record
{ _≈_ = λ (fx fy : F Carrier) → ∀ (l : Log) → index fx l ≈ index fy l
; isEquivalence = record
{ refl = λ _ → refl
; sym = λ eq l → sym (eq l)
; trans = λ i≈j j≈k l → trans (i≈j l) (j≈k l)
}
}
module FS = Setoid FS
field
-- tabulate-index : (fx : F Carrier) → tabulate (index fx) FS.≈ fx
index-tabulate : (f : Log → Carrier) (l : Log) → index (tabulate f) l ≈ f l
tabulate-index : (fx : F Carrier) → tabulate (index fx) FS.≈ fx
tabulate-index = index-tabulate ∘ index
PropositionalNaperian : Set (suc (a ⊔ c) ⊔ b)
PropositionalNaperian = ∀ A → Naperian (≡.setoid A) |
Now, after such (I think: semantics-preserving!) rearrangement, we can observe that the parametrisation on As things stand, Hope that the suggested refactoring make sit easier to see a possible way forward, but as I've critiqued the previous design in this style, a shout-out to @JacquesCarette for comment on this latest iteration. |
Re: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lots that can be (further) simplified, but I still think that the parametrisation on Setoid
needs fixing!
src/Effect/Functor/Naperian.agda
Outdated
module Effect.Functor.Naperian where | ||
|
||
open import Effect.Functor using (RawFunctor) | ||
open import Function.Bundles using (_⟶ₛ_; _⟨$⟩_; Func) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
open import Function.Bundles using (_⟶ₛ_; _⟨$⟩_; Func) | |
open import Function.Base using (_∘_) |
src/Effect/Functor/Naperian.agda
Outdated
open import Function.Bundles using (_⟶ₛ_; _⟨$⟩_; Func) | ||
open import Level using (Level; suc; _⊔_) | ||
open import Relation.Binary.Bundles using (Setoid) | ||
open import Relation.Binary.PropositionalEquality.Core using (_≡_) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The relation itself is now never used, only its property of admitting a Setoid
bundle, so this can be deleted
open import Relation.Binary.PropositionalEquality.Core using (_≡_) |
src/Effect/Functor/Naperian.agda
Outdated
open import Level using (Level; suc; _⊔_) | ||
open import Relation.Binary.Bundles using (Setoid) | ||
open import Relation.Binary.PropositionalEquality.Core using (_≡_) | ||
open import Relation.Binary.PropositionalEquality.Properties as ≡ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can simplify this to:
open import Relation.Binary.PropositionalEquality.Properties as ≡ | |
import Relation.Binary.PropositionalEquality.Properties as ≡ using (setoid) |
src/Effect/Functor/Naperian.agda
Outdated
|
||
private | ||
variable | ||
a b c e f : Level |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Standardise, based on parametrisation on S : Setoid a ℓ
, itself a lexical convention:
a b c e f : Level | |
a b c ℓ : Level |
src/Effect/Functor/Naperian.agda
Outdated
-- module Propositional where | ||
-- record Naperian {F : Set a → Set b} c (RF : RawFunctor F) : Set (suc (a ⊔ c) ⊔ b) where | ||
-- field | ||
-- rawNaperian : RawNaperian c RF | ||
-- open RawNaperian rawNaperian public | ||
-- field | ||
-- tabulate-index : (fa : F A) → tabulate (index fa) ≡ fa | ||
-- index-tabulate : (f : Log → A) → ((l : Log) → index (tabulate f) l ≡ f l) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete!
src/Effect/Functor/Naperian.agda
Outdated
module _ {F : Set a → Set b} c (RF : RawFunctor F) where | ||
private | ||
FA : (S : Setoid a e) → (rn : RawNaperian c RF) → Setoid b (c ⊔ e) | ||
FA S rn = record | ||
{ _≈_ = λ (fx fy : F Carrier) → (l : Log) → index fx l ≈ index fy l | ||
; isEquivalence = record | ||
{ refl = λ _ → refl | ||
; sym = λ eq l → sym (eq l) | ||
; trans = λ i≈j j≈k l → trans (i≈j l) (j≈k l) | ||
} | ||
} | ||
where | ||
open Setoid S | ||
open RawNaperian rn | ||
|
||
record Naperian (S : Setoid a e) : Set (suc a ⊔ b ⊔ suc c ⊔ e) where | ||
field | ||
rawNaperian : RawNaperian c RF | ||
open RawNaperian rawNaperian public | ||
private | ||
module FS = Setoid (FA S rawNaperian) | ||
module A = Setoid S | ||
field | ||
tabulate-index : (fx : F A.Carrier) → tabulate (index fx) FS.≈ fx | ||
index-tabulate : (f : Log → A.Carrier) → ((l : Log) → index (tabulate f) l A.≈ f l) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the revised version in my extended comment.
src/Effect/Functor/Naperian.agda
Outdated
index-tabulate : (f : Log → A.Carrier) → ((l : Log) → index (tabulate f) l A.≈ f l) | ||
|
||
PropositionalNaperian : Set (suc (a ⊔ c) ⊔ b) | ||
PropositionalNaperian = ∀ {A} → Naperian (≡.setoid A) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the current design wrt parametrisation, I think that A
should be explicit:
PropositionalNaperian = ∀ {A} → Naperian (≡.setoid A) | |
PropositionalNaperian = ∀ A → Naperian (≡.setoid A) |
Yes, it would make sense to 'restart' the work on |
Thanks for the review @jamesmckinna . All the explicit suggestions should go in - please do so @gabriellisboaconegero . |
src/Effect/Functor/Naperian.agda
Outdated
natural-tabulate : (f : Carrier → Carrier) (k : Log → Carrier) → (tabulate (f ∘ k)) FS.≈ (f <$> (tabulate k)) | ||
natural-index : (f : Carrier → Carrier) (as : F Carrier) (l : Log) → (index (f <$> as) l) ≈ f (index as l) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Can natural-tabulate
also be eliminated by defining it in terms of index-tabulate
, natural-index
, and tabulate-index
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it can without assuming congruence on the setoid. I was trying to do so, but I was stuck and needed to prove the function congruence on the setoid S
. I could figure out that
natural-index f (tabulate k) l -> index (f <$> tabulate k) l ≈ f (index (tabulate k) l)
index-tabulate k l -> index (tabulate k) l ≈ k l
index-tabulate (f ∘ k) l -> index (tabulate (λ x → f (k x))) l ≈ f (k l)
If I assume ≈
congruence, then I could prove tabulate-index
.
I don't think we can find a proof for tabulate-index
without congruence, but I am not sure. I base my argument on the fact that tabulate-∘
needs cong
to be proved.
src/Data/Vec/Effectful.agda
Outdated
{ _<$>_ = map | ||
} | ||
|
||
naperian : RawNaperian (λ (A : Set a) → Vec A n) 0ℓ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I would name this rawNaperian
and the one below naperian
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly: rawApplicative
below... etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the naming of naperian
was easy. However, changing applicative
to rawApplicative
caused problems with the module TraversableM
, as it attempts to open the rawApplicative
inside the RawMonad
, which can conflict with the naming.
The way I found was to explicitly extract the rawApplicative
from RawMonad
with open TraversableA (RawMonad.rawApplicative Mon) public
src/Data/Vec/Effectful.agda
Outdated
fullNaperian : PropositionalNaperian (λ (A : Set a) → Vec A n) 0ℓ | ||
fullNaperian A = record | ||
{ rawNaperian = naperian | ||
; index-tabulate = λ f l → lookup∘tabulate f l |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eta contract?
src/Effect/Functor/Naperian.agda
Outdated
Naperian-Applicative : RawNaperian → RawApplicative F | ||
Naperian-Applicative rn = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naperian-Applicative : RawNaperian → RawApplicative F | |
Naperian-Applicative rn = | |
rawApplicative : RawNaperian → RawApplicative F | |
rawApplicative rn = |
on our 'usual' naming model?
Nice to see the |
Is this the 'parametrisation' comment above, or something else? I did discuss changing some of that with @gabriellisboaconegero last week so that |
yes, this is the parametrisation comment, but I don't think it is solved by changing the parametrisation(s) of But it's entirely possible that I have misunderstood something fundamental, so I'll wait and see! It (perhaps) doesn't help that #496 is still open, and that we haven't (really) resolved how a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully this is what @jamesmckinna meant!
I'll try to look at this over the w/e, but I wonder if it's also worth soliciting a review from @gallais who did a lot of the early work on Meanwhile, I from time to time meet up with Hancock himself, so I'll also try to discuss this with him! |
This pull request aims to continue and complete the work started in #2004
The first version already includes changes requested by @jamesmckinna, such as correct stdlib naming variables and redefinition of
PropositionalNaperian
.