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

foldMap, foldMap' for PrimArray #186

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
12 changes: 12 additions & 0 deletions Data/Primitive/PrimArray.hs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ module Data.Primitive.PrimArray
, foldlPrimArray
, foldlPrimArray'
, foldlPrimArrayM'
, foldMapPrimArray
, foldMapPrimArray'
-- * Effectful Folding
, traversePrimArray_
, itraversePrimArray_
Expand Down Expand Up @@ -467,6 +469,16 @@ sizeofPrimArray :: forall a. Prim a => PrimArray a -> Int
{-# INLINE sizeofPrimArray #-}
sizeofPrimArray (PrimArray arr#) = I# (quotInt# (sizeofByteArray# arr#) (sizeOf# (undefined :: a)))

-- | Lazily map each element of the primitive array to a monoid, and combine the results.
foldMapPrimArray :: forall a m. (Prim a, Monoid m) => (a -> m) -> PrimArray a -> m
Copy link
Collaborator

Choose a reason for hiding this comment

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

The documentation is insufficient. It should explain how the combination associates. In fact, I think we should almost surely offer more than one version: associating to the left, associating to the right, and (for certain tree-like monoids) associating in a balanced fashion are all potentially useful.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Tree-like or in some cases numerical. Balanced association can reduce floating point errors somewhat, for example.

{-# INLINE foldMapPrimArray #-}
foldMapPrimArray f = foldrPrimArray (\a acc -> acc `mappend` f a) mempty
chessai marked this conversation as resolved.
Show resolved Hide resolved

-- | Strictly map each element of the primitive array to a monoid, and combine the results.
foldMapPrimArray' :: forall a m. (Prim a, Monoid m) => (a -> m) -> PrimArray a -> m
Copy link
Collaborator

Choose a reason for hiding this comment

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

The documentation is insufficient. It should explain which way the combination associates. Also, it's notably not (necessarily) strict in the function results. The documentation should reflect this and there should be a comment explaining that decision.

{-# INLINE foldMapPrimArray' #-}
foldMapPrimArray' f = foldlPrimArray' (\ !acc x -> acc `mappend` f x) mempty

-- | Lazy right-associated fold over the elements of a 'PrimArray'.
{-# INLINE foldrPrimArray #-}
foldrPrimArray :: forall a b. Prim a => (a -> b -> b) -> b -> PrimArray a -> b
Expand Down
4 changes: 3 additions & 1 deletion changelog.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
## Changes in version 0.6.4.1

* Add instances for the following newtypes from `base`:
* Add `foldMapPrimArray` and `foldMapPrimArray'`
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to be updated now that you've added more functions.


* Add `Prim` instances for the following newtypes from `base`:
`Const`, `Identity`, `Down`, `Dual`, `Sum`, `Product`,
`First`, `Last`, `Min`, `Max`

Expand Down