Skip to content

A more robust implementation of Replicator.elim#390

Merged
aspiwack merged 4 commits intomasterfrom
more-robust-elim
Feb 21, 2022
Merged

A more robust implementation of Replicator.elim#390
aspiwack merged 4 commits intomasterfrom
more-robust-elim

Conversation

@aspiwack
Copy link
Copy Markdown
Member

@aspiwack aspiwack commented Feb 17, 2022

I rely on more type families (I resurrected the FunN type family,
for instance) and additional type classes in order to give robust
hints to the type checker to infer some of the arguments based on the
others in multiple possible ways.

The previous version was abusing functional dependencies to that
effect. The problem is that it almost certainly depended on bugs. Plus
some behaviour changed in 9.2 making the recursive instance not
typecheck anymore.

The actual (staging) recursion now uses (type-level) Peano integers,
this way we can get rid of the OVERLAPPABLE instances altogether,
and avoid the sort of fragile behaviour that caused the failure in
9.2.

The 'elim' function itself has more type arguments, so I took it out
of the type class. A collection of type families and type classes fill
the role that functional dependencies used to play: figuring out f
from n or n from f.

Fixes #388.


Interestingly I've only had to fix Replicator.elim for the sake of
fixing the compilation in GHC 9.2. The rest works fine. But it'd be
better to make similar changes to V.make and V.elim. I won't have
time to, though, @tbagrel1 can I untrust you with this?

I've made some additional small changes for the sake of GHC 9.2
compilation:

Minimal shell.nix changes to be able to run GHC 9.2

Some 𝜂-expansions to compile with GHC 9.2

This is really suspicious, there may be a bug in GHC 9.2 where
sections do not preserve linearity. Grmpf.

@tbagrel1
Copy link
Copy Markdown
Member

I will take a look at it tomorrow morning!

Copy link
Copy Markdown
Member

@tbagrel1 tbagrel1 left a comment

Choose a reason for hiding this comment

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

I just have questions at the moment (see review comments)

-- has to solve. Look in particular at the instance @'IsFunN' a b (a' %p ->
-- f))@: it matches liberally, so triggers on quite underdetermined `f`, but has
-- equality constraints in its context which will help the type checker.
class IsFunN a b f
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Couldn't we use something like

class IsFunN a b f where
  type Arity

instance IsFunN a b b where
  type Arity = 'Z

instance (IsFunN a b f, a' ~ a, p ~ 'One) => IsFunN a b (a' %p -> f) where
  type Arity = 'S (Arity @a @b @f)

to get rid of the explicit Arity type family ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I like the idea of having one class FunN to infer f from n a b, and one class IsFunN to infer n from a b f. Cheking IsFunN a b f should allow to compute the arity along the way (at least theorically).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Did you manage to answer your own question? if so, I'm just waiting for the green tick so that I can merge.

Copy link
Copy Markdown
Member

@tbagrel1 tbagrel1 Feb 21, 2022

Choose a reason for hiding this comment

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

Yes, sorry
I thought that you wanted me to continue the work on this branch.

Copy link
Copy Markdown
Member Author

@aspiwack aspiwack Feb 21, 2022

Choose a reason for hiding this comment

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

I thought that you wanted me to continue the work on this branch.

I'll pretty never be asking you to continue work on a branch which passes tests 🙂 . If something is mergeable as it is, then we merge it. Merging small chunks early makes everybody's life easier.

Use as `nix-shell --argstr "ghcVersion" "921"`.
This is really suspicious, there may be a bug in GHC 9.2 where
sections do not preserve linearity. Grmpf.
I rely on more type families (I resurrected the `FunN` type family,
for instance) and additional type classes in order to give robust
hints to the type checker to infer some of the arguments based on the
others in multiple possible ways.

The previous version was abusing functional dependencies to that
effect. The problem is that it almost certainly depended on bugs. Plus
some behaviour changed in 9.2 making the recursive instance not
typecheck anymore.

The actual (staging) recursion now uses (type-level) Peano integers,
this way we can get rid of the `OVERLAPPABLE` instances altogether,
and avoid the sort of fragile behaviour that caused the failure in
9.2.

The 'elim' function itself has more type arguments, so I took it out
of the type class. A collection of type families and type classes fill
the role that functional dependencies used to play: figuring out `f`
from `n` or `n` from `f`.

Fixes #388.
Copy link
Copy Markdown
Member

@tbagrel1 tbagrel1 left a comment

Choose a reason for hiding this comment

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

After some time to process it, it looks good to me.

@aspiwack aspiwack merged commit 151241e into master Feb 21, 2022
@aspiwack aspiwack deleted the more-robust-elim branch February 21, 2022 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Data.Replicator.Linear.Internal doesn't compile under GHC 9.2

3 participants