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

Add new module Effect.Functor.Naperian #2004

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

Sofia-Insa
Copy link
Contributor

@Sofia-Insa Sofia-Insa commented Jun 22, 2023

New module Effect.Functor.Naperian added to stdlib.
With two records defining Naperian functors:

  • record RawNaperian
  • record Naperian.

Sofia-Insa and others added 4 commits June 21, 2023 16:52
with two records :
`RawNaperian` and `Naperian`
Added new file `Effect.Functor.Naperian` and two records to it
Copy link
Contributor

@jamesmckinna jamesmckinna left a comment

Choose a reason for hiding this comment

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

A very nice initial go at a contribution in this area, and hopefully one which can be taken further as suggested, without too much difficulty.

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
src/Effect/Functor/Naperian.agda Outdated Show resolved Hide resolved
src/Effect/Functor/Naperian.agda Outdated Show resolved Hide resolved

-- " Functor f is Naperian if there is a type p of ‘positions’ such that fa≃p→a;
-- then p behaves a little like a logarithm of f
-- in particular, if f and g are both Naperian, then Log(f×g)≃Logf+Logg and Log(f.g) ≃ Log f × Log g"
Copy link
Contributor

Choose a reason for hiding this comment

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

So it would be great if your PR could come good on this promise by exhibiting the constructions?

src/Effect/Functor/Naperian.agda Outdated Show resolved Hide resolved
src/Effect/Functor/Naperian.agda Outdated Show resolved Hide resolved
rawNaperian : RawNaperian d RF
open RawNaperian rawNaperian public
field
tabulate-index : ∀ {A} → (fa : F A) → tabulate (index fa) ≡ fa
Copy link
Contributor

Choose a reason for hiding this comment

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

So: here the induced Setoid relation on F A?

src/Effect/Functor/Naperian.agda Outdated Show resolved Hide resolved
@JacquesCarette
Copy link
Contributor

I should take this PR over. I'm not entirely sure I know how?

@MatthewDaggitt
Copy link
Contributor

MatthewDaggitt commented Mar 17, 2024

Given the comment of @JacquesCarette above, @jamesmckinna would you be willing to take over this? Otherwise, I'll close it.

@jamesmckinna
Copy link
Contributor

jamesmckinna commented Mar 17, 2024

@MatthewDaggitt @JacquesCarette happy to take over on this one, but it'll be a while before I return proper attention to it, if that's OK?

@jamesmckinna
Copy link
Contributor

Minimal cleanup for the time being.

@jamesmckinna jamesmckinna marked this pull request as draft March 17, 2024 15:36
@jamesmckinna jamesmckinna changed the title Upload new file Effect.Functor.Naperian to stdlib Add new module Effect.Functor.Naperian Mar 17, 2024
@JacquesCarette
Copy link
Contributor

Thanks a lot! I've been wanting to come back to these, but I've barely found enough time to do a few reviews.

@JacquesCarette
Copy link
Contributor

This is now back onto my plate to finish. And I even know how to do so!

@jamesmckinna
Copy link
Contributor

This is now back onto my plate to finish. And I even know how to do so!

Fantastic! I'm happy to hand the baton back. But sign me up as a reviewer come the right time...

Copy link
Contributor

@jamesmckinna jamesmckinna left a comment

Choose a reason for hiding this comment

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

Besides the notational nitpicking (which I think important, but advisory here), I think the design needs some more thinking as to the dependency/parametrisation in the Setoid case.

Comment on lines +6 to +9
-- Definitions of Naperian Functors, as named by Hancock and McBride,
-- and subsequently documented by Jeremy Gibbons
-- in the article "APLicative Programming with Naperian Functors"
-- which appeared at ESOP 2017.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
-- Definitions of Naperian Functors, as named by Hancock and McBride,
-- and subsequently documented by Jeremy Gibbons
-- in the article "APLicative Programming with Naperian Functors"
-- which appeared at ESOP 2017.
-- Naperian Functors, as defined by Hancock and McBride, and
-- subsequently documented by Jeremy Gibbons in the paper
-- "APLicative Programming with Naperian Functors" (ESOP 2017).

Comment on lines +41 to +51
-- Full Naperian has the coherence conditions too.
-- Propositional version (hard to work with).

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)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is Propositional.Naperian definable directly from Naperian (≡.setoid A) below?

Comment on lines +55 to +66
FA : (AS : Setoid a e) → (rn : RawNaperian c RF) → Setoid b (c ⊔ e)
FA AS rn = record
{ Carrier = F X
; _≈_ = λ fx fy → (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 AS renaming (Carrier to X)
Copy link
Contributor

Choose a reason for hiding this comment

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

Conventionally, in the stdlib

  • S is the generic Setoid; AS seems weird here
  • open Setoid S renaming (Carrier to A), ditto.
  • the Carrier field is (usually) not mentioned; I might be tempted just to write
record
      { _≈_ = λ (fx fy : F A)  (l : Log)  index fx l ≈ index fy l
...

but YMMV

Comment on lines +74 to +75
module FS = Setoid (FA AS rawNaperian)
module A = Setoid AS
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar comments to the above about 'conventional' naming?

Comment on lines +69 to +72
record Naperian (AS : Setoid a e) : Set (suc a ⊔ b ⊔ suc c ⊔ e) where
field
rawNaperian : RawNaperian c RF
open RawNaperian rawNaperian public
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the one area of the design with which I strongly diverge from your choices: this definition makes the RawNaperian structure potentially depend(ent) on the argument index AS, and that for me is a deal-breaker...

... so I'd be interested to learn the rationale for this choice!

private
variable
a b c e f : Level
A : Set a
Copy link
Contributor

Choose a reason for hiding this comment

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

If this choice causes ambiguity problems with opening Setoid etc. below, then suggest removing it in favour of ∀ {A} → ... in the types of index and tabulate below...

@jamesmckinna
Copy link
Contributor

The PR is still DRAFT, so I'll revisit/re-review when it's 'ready', but at that point I'll also revisit the early comment above about proving the 'logarithm' properties.

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

Successfully merging this pull request may close these issues.

4 participants