Port to cats

[rossabaker]

Two issues that need to be sorted through, but this can start the discussion:

• GDecomp[N,?,B] does not satisfy Cats' Comonad laws.

• Cats doesn't have a Tree. Not sure if we can replace this with Cofree[Stream,?]. None of the functions that use Tree have test coverage, so everything is commented for now.

Fixes #19.

[adelbertc]

@rossabaker I wonder if your GDecomp Comonad failure can be fixed by my fix here: #26

[rossabaker]

Thanks! I merged #26 into my branch, and it's still failing for me on two laws introduced by Cats that aren't in Scalaz:

[info] ! Graph.coflatMap.coflatten throughMap: Falsified after 7 passed tests.
[info] > Labels of failing property:
[info] (GDecomp(Context(Vector((-2147483648,-128), (0,124), (2147483647,1), (-1,-6), (-2147483648,18), (2147483647,0), (652777520,-128)),-4,GDecomp(Context(Vector((-2147483648,-128), (0,124), (2147483647,1), (-1,-6), (-2147483648,18), (2147483647,0), (652777520,-128)),-4,GDecomp(Context(Vector((-2147483648,-128), (0,124), (2147483647,1), (-1,-6), (-2147483648,18), (2147483647,0), (652777520,-128)),-4,-1,Vector((-1,127), (-1,-53), (2147483647,-1), (-1,-18), (1,0))),1:484214008->[]),Vector((-1,127), (-1,-53), (2147483647,-1), (-1,-18), (1,0))),1:GDecomp(Context(Vector(),1,484214008,Vector((2147483647,-4))),-4:-1->[(1,0),(-1,-18),(2147483647,-1),(-1,127),(-1,-53)])->[]),Vector((-1,127), (-1,-53), (2147483647,-1), (-1,-18), (1,0))),1:GDecomp(Context(Vector(),1,GDecomp(Context(Vector(),1,484214008,Vector((2147483647,-4))),-4:-1->[(1,0),(-1,-18),(2147483647,-1),(-1,127),(-1,-53)]),Vector((2147483647,-4))),-4:GDecomp(Context(Vector((-2147483648,-128), (0,124), (2147483647,1), (-1,-6), (-2147483648,18), (2147483647,0), (652777520,-128)),-4,-1,Vector((-1,127), (-1,-53), (2147483647,-1), (-1,-18), (1,0))),1:484214008->[])->[(1,0),(-1,-18),(2147483647,-1),(-1,127),(-1,-53)])->[]) ?== GDecomp(Context(Vector((-2147483648,-128), (0,124), (2147483647,1), (-1,-6), (-2147483648,18), (2147483647,0), (652777520,-128)),-4,GDecomp(Context(Vector((-2147483648,-128), (0,124), (2147483647,1), (-1,-6), (-2147483648,18), (2147483647,0), (652777520,-128)),-4,GDecomp(Context(Vector((-2147483648,-128), (0,124), (2147483647,1), (-1,-6), (-2147483648,18), (2147483647,0), (652777520,-128)),-4,-1,Vector((-1,127), (-1,-53), (2147483647,-1), (-1,-18), (1,0))),1:484214008->[]),Vector((-1,127), (-1,-53), (2147483647,-1), (-1,-18), (1,0))),1:GDecomp(Context(Vector(),1,484214008,Vector((2147483647,-4))),-4:-1->[(1,0),(-1,-18),(2147483647,-1),(-1,127),(-1,-53)])->[]),Vector((-1,127), (-1,-53), (2147483647,-1), (-1,-18), (1,0))),1:GDecomp(Context(Vector(),1,GDecomp(Context(Vector(),1,484214008,Vector((2147483647,-4))),-4:-1->[(1,0),(-1,-18),(2147483647,-1),(-1,127),(-1,-53)]),Vector((2147483647,-4))),-4:GDecomp(Context(Vector((2147483647,0), (-2147483648,-128), (652777520,-128), (2147483647,1), (0,124), (-2147483648,18), (-1,-6)),-4,-1,Vector((1,0), (-1,-18), (2147483647,-1), (-1,127), (-1,-53))),1:484214008->[])->[(1,0),(-1,-18),(2147483647,-1),(-1,127),(-1,-53)])->[])) failed
[info] > ARG_0: GDecomp(Context(Vector((-2147483648,-128), (0,124), (2147483647,1), (-1,-6), (-2147483648,18), (2147483647,0), (652777520,-128)),-4,-1,Vector((-1,127), (-1,-53), (2147483647,-1), (-1,-18), (1,0))),1:484214008->[])

[info] ! Graph.coflatMap.coflatMap associativity: Falsified after 6 passed tests.
[info] > Labels of failing property:
[info] (GDecomp(Context(Vector((2147483647,-15), (-1875057409,-128), (-867742067,1), (-1,1)),0,-91560092,Vector((955748330,74), (-1425857438,0), (1,99), (-1414313717,1), (1,127))),25:-953347883->[]
[info] -19:2147483647->[]
[info] 127:-73163584->[]
[info] -112:1->[]) ?== GDecomp(Context(Vector((2147483647,-15), (-1875057409,-128), (-867742067,1), (-1,1)),0,-91560092,Vector((955748330,74), (-1425857438,0), (1,99), (-1414313717,1), (1,127))),25:1->[]
[info] -19:-1->[]
[info] 127:-902385978->[]
[info] -112:806232476->[])) failed
[info] > ARG_0: GDecomp(Context(Vector((2147483647,-15), (-1875057409,-128), (-867742067,1), (-1,1)),0,-245313765,Vector((955748330,74), (-1425857438,0), (1,99), (-1414313717,1), (1,127))),25:1743515143->[]
[info] -19:0->[]
[info] 127:0->[]
[info] -112:1->[])
[info] > ARG_1: org.scalacheck.GenArities$$Lambda$631/43326528@1666860a
[info] > ARG_2: org.scalacheck.GenArities$$Lambda$631/43326528@32a4bdeb

The Order instances I needed to introduce to get a Cogen make me uneasy. I've not yet had enough time to concentrate on whether I created bad instances, or whether the fact we need Orders on Sets and Maps at all is responsible for this problem. The laws that are present in Scalaz pass on the Cats port with or without #26.

[adelbertc]

I suppose an easy way of making your Order instances ever so slightly less sketchy is to sort them before returning.

[joroKr21]

I think I have identified the problems revealed by the comonad laws (there's quite a few of them):

1. GDecomp.redecorate - fixed in Fix GDecomp#redecorate #26

2. GraphGen.genGDecomp - not yet fixed in Fix GDecomp#redecorate #26 (solution in this comment)

3. Cogen.cogenMap - should not require ordering on the values (Cogen.cogenMap should not require ordering on the values typelevel/scalacheck#356)

4. (c: Context[N, A, B]) => (c.toGrContext, c.vertex) and (gc: GrContext[N, A, B], v: N) => gc.toContext(v) don't form a bijection, because Context stores adjacent nodes in a Vector whereas GrContext stores them in a Map. At the same time the function expected by Graph.gmap operates on Context so it could make (wrong) assumptions about the order of the adjacent nodes. This is reflected in the Order and Cogen instances for Context. I'm not sure what is the best way to fix this. Some possible options:

   1. Store adjacent nodes in Context in a Map as in GrContext
   2. Sort adjacent node vectors in Context (would require an Ordering on N and B)
   3. Change Graph.gmap to operate on GrContext (problem - the focused vertex is missing)
   4. Change the Order and Cogen definitions for Context to disregard the order of adjacent nodes (however that means making assumptions about the behaviour of functions passed to Graph.gmap).

[joroKr21]

I sketched the quick and dirty solution to 4. (iv.) and the comonad laws are satisfied here: joroKr21/quiver@3606546

[rossabaker]

Thanks, @joroKr21! @mgttlinger is still on Scalaz, so we're going to get #23, #24, and #26 assembled in as quiver-6 (#27), and then I will incorporate your changes here on top of that as quiver-7.

[joroKr21]

@rossabaker Feel free to do that. But maybe we should open a separate issue about Context, GrContext, gmap and the order of the adjacency list. Or if this is by design, somehow reflect it in the docs. E.g. if I call .foldLeft over all adjacent nodes in a gmap transformation, I better make sure it's commutative.

[rossabaker]

Okay, finally getting back to this. I'd like to get this working on cats-1.0 and release quickly after cats' 1.0 release next week.

I've rebased to get the latest from master, and cherry-picked @joroKr21's change to restore the Comonad. I'm out of energy for tonight, but for cats-1.0, it looks like Order.on is the biggest problem.

[joroKr21]

Looks like those methods were moved to the companion objects in typelevel/cats#1997 but other than that they should be the same.

[rossabaker]

We should be in good shape here to release when cats-1.0 is available.

Are we all comfortable with the current solution to the ordering, or does someone want to make a late push for option i or ii?

[rossabaker]

These are falsely getting flagged as unused in certain versions of Scala. I believe they will be unnecessary after a new release of scalacheck.

This is sloppy, but since it doesn't affect the public API, I'm content to live with it.

[joroKr21]

cogenMap won't be necessary, but cogenContext will be because of the no-ordering assumption

[joroKr21]

👍 for the current solution, even if it means being somewhat hand-wavy.

Option I. would trigger a cascade of API changes because of the pervasive use of Vector where no ordering guarantee is given (starting from predecessors, successors, neighbors, etc.)

I don't like option II. either, because it means constraining graphs only to elements with a defined Order.

[rossabaker]

Okay, this is upgraded to cats-1.0.0-RC2. I propose releasing this as quiver-7, and we'll follow with a patch next week when cats-1.0.0 is available.