haskell / vector

An efficient implementation of Int-indexed arrays (both mutable and immutable), with a powerful loop optimisation framework .
Other
366 stars 139 forks source link

A desirable Boxed instance for Unboxed #508

Closed recursion-ninja closed 1 month ago

recursion-ninja commented 1 month ago

This PR contains a proof of concept for Unbox instances of "boxed" data-types by way of the newly added AsBoxedStrictly and AsBoxedLazily newtypes. This PR is intended to address issue #503.

The AsBoxedStrictly newtype imposes new strictness semantics on the underlying type, ensuring the wrapped value is always reduced to normal form. This invariant requires an NFData instance for the underlying type. Furthermore, it requires the use of a "smart constructor" which evaluates the wrapped value to normal form, since a new-type cannot have strictness annotations. Hence there are makeBoxedStrictly and grabBoxedStrictly exposed from the Data.Vector.Unboxed module to construct and deconstruct AsBoxedStrictly values, respectively.

The AsBoxedLazily new-type is much simpler since it preserves the strictness semantics of the underlying data-type unaltered.

Here are two example usages of the newtypes as a proof of concept:

Strict usage

data Foo a = Foo Int a
  deriving (Eq, Ord, Show)

instance NFData a => VU.IsoUnbox (Foo a) (Int, AsBoxedStrictly a) where
  toURepr (Foo i a) = (i, makeBoxedStrictly a)
  fromURepr (i, a) = Foo i $ grabBoxedStrictly a
  {-# INLINE toURepr #-}
  {-# INLINE fromURepr #-}

newtype instance VU.MVector s (Foo a) = MV_Foo (VU.MVector s (Int, AsBoxedStrictly a))

newtype instance VU.Vector    (Foo a) = V_Foo  (VU.Vector    (Int, AsBoxedStrictly a))

deriving via (Foo a `VU.As` (Int, AsBoxedStrictly a)) instance NFData a => VGM.MVector VUM.MVector (Foo a)

deriving via (Foo a `VU.As` (Int, AsBoxedStrictly a)) instance NFData a => VG.Vector   VU.Vector   (Foo a)

instance NFData a => VU.Unbox (Foo a)

In GHCi

import qualified Data.Vector.Unboxed as VU
VU.fromListN 3 [ Foo 4 "Hello", Foo 8 "there", Foo 16 "sailor" ]
[Foo 4 "Hello",Foo 8 "there",Foo 16 "sailor"]

Lazy usage

data Bar a = Bar Int a
  deriving (Eq, Ord, Show)

instance VU.IsoUnbox (Bar a) (Int, AsBoxedLazily a) where
  toURepr (Bar i a) = (i, AsBoxedLazily a)
  fromURepr (i, AsBoxedLazily a) = Bar i a
  {-# INLINE toURepr #-}
  {-# INLINE fromURepr #-}

newtype instance VU.MVector s (Bar a) = MV_Bar (VU.MVector s (Int, AsBoxedLazily a))

newtype instance VU.Vector    (Bar a) = V_Bar  (VU.Vector    (Int, AsBoxedLazily a))

deriving via (Bar a `VU.As` (Int, AsBoxedLazily a)) instance VGM.MVector VUM.MVector (Bar a)

deriving via (Bar a `VU.As` (Int, AsBoxedLazily a)) instance VG.Vector   VU.Vector   (Bar a)

instance VU.Unbox (Bar a)

In GHCi

import qualified Data.Vector.Unboxed as VU
VU.fromListN 3 [ Bar 3 "Bye", Bar 2 "for", Bar 1 "now" ]
[Bar 3 "Bye",Bar 2 "for",Bar 1 "now"]

Nota Bene

Since this is a proof of concept, all types and functions names are subject to change after the appropriate level of "bike-shedding" has occurred. Commentary is welcomed and encouraged.

Shimuuar commented 1 month ago

I haven't looked at the code yet. CI is broken at the moment fix should be in #507

Shimuuar commented 1 month ago

@recursion-ninja If you're trying to fix doctest it's easier to do so locally. Run in vector subdir.

$ cabal build all --write-ghc-environment-files=always
$ cabal run -- vector-doctest
recursion-ninja commented 1 month ago

@recursion-ninja If you're trying to fix doctest it's easier to do so locally. Run in vector subdir.

$ cabal build all --write-ghc-environment-files=always
$ cabal run -- vector-doctest

Yes, I was. This is quite helpful.

recursion-ninja commented 1 month ago

The test now pass, but I still need to do a pass over the documentation.

recursion-ninja commented 1 month ago

The test now pass, but I still need to do a pass over the documentation.

I have updated the documentation.

@Shimuuar I think the PR is ready for review, and and potentially merged if the package maintainers agree.

Shimuuar commented 1 month ago

Other than comment above PR is good to go.

I'm not sure why CI is failing. It seems to have stuck for some reason and then job got cancelled.

recursion-ninja commented 1 month ago

Other than comment above PR is good to go.

@Shimuuar I addressed the comment above. Should be good to merge now.

Shimuuar commented 1 month ago

Merged. @recursion-ninja Thank you!