search

HuwCampbell / grenade

Deep Learning in Haskell
BSD 2-Clause "Simplified" License
1.44k stars 84 forks source link

Update ghcs #72

Closed tmcgilchrist closed 5 years ago

tmcgilchrist commented 5 years ago

A few changes in here:

On the last point I'm not entirely convinced I've chosen the right source of natVal. An example of the compile error is:

 test/Test/Grenade/Network.hs:364:40: error:
    Ambiguous occurrence ‘natVal’
    It could refer to either ‘Data.Singletons.TypeLits.natVal’,
                             imported from ‘Data.Singletons.TypeLits’ at test/Test/Grenade/Network.hs:24:1-41
                             (and originally defined in ‘GHC.TypeNats’)
                          or ‘GHC.TypeLits.natVal’,
                             imported from ‘GHC.TypeLits’ at test/Test/Grenade/Network.hs:35:1-29
    |
364 |                         let output_f = natVal f
    |                                        ^^^^^^

with the two type signatures being:

The change to Natural in singletons causes some issues in the test cases, which is why the tests use GHC.TypeLits.natVal rather than the singletons version.

HuwCampbell commented 5 years ago

Thanks Tim!

I think this might be problematic though, it looks like the singletons library switched its export of natval from GHC.TypeLits to GHC.TypeNats in version 2.4. So This is going to pull in different definitions depending on which compiler one is using.

The TypeNats one has existed for long enough, so maybe we should just swap to that for all the compilers.

tmcgilchrist commented 5 years ago

So digging through base looks like GHC.TypeNats came in Since: base-4.10.0.0, which means ghc 8.2.1 and above 1.

erikd commented 5 years ago

I didn't see this PR, but i fixed everything to build with 8.4.3 in #75

HuwCampbell commented 5 years ago

Thanks guys.

I'm still a bit iffy on using the Natural version in 8.2 and above, and the Integer version in 7.10 and 8.0; but I'm not sure there's a great way around that apart from dropping support for the older ones.

I'll merge the other PR. I would prefer to use Natural going forwards.

erikd commented 5 years ago

:+1: