haskell / primitive

This package provides various primitive memory-related operations.
Other
114 stars 58 forks source link

fromList for mutable arrays #413

Open meooow25 opened 2 months ago

meooow25 commented 2 months ago

It would useful to have a function like

mutableArrayFromListN :: PrimMonad m => Int -> [a] -> m (MutableArray (PrimState m) a)

given that there already exists

arrayFromListN :: Int -> [a] -> Array a 

My particular use case is also more directly expressed by generate, but I understand if this is too much for this library.

generate :: PrimMonad m => Int -> (Int -> a) -> m (MutableArray (PrimState m) a)
andrewthad commented 2 months ago

I think this is pretty reasonable, particularly because the variant returning a mutable array is the more fundamental one. The immutable variants are just the mutable variants but with the result frozen:

generate n f = runST (unsafeFreeze (generateMutable n f))
arrayFromListN n xs = runST (unsafeFreeze (mutableArrayFromListN n xs))

I'm would take a PR adding the new functions, and the existing (immutable) functions should be redefined to use the mutable variants in their implementation.

lehins commented 2 months ago

I would recommend creating a function that can fill a mutable array that was passed as an argument, rather than returning a new mutable array.

This will be more flexible. You can create one mutable array and fill different parts of it with different lists, potentially even in parallel.

meooow25 commented 2 months ago

While that would be more flexible, I cannot recall any instance where I wanted to fill an existing array with elements from a list. I am also not aware of any existing library that offers such a function.

However, vector has generate and generateM, and array has newListArray and newGenArray.

lehins commented 2 months ago

It is easy to implement mutableArrayFromListN in terms of fillMutableArrayWithListListN:

mutableArrayFromListN :: PrimMonad m => Int -> [a] -> m (MutableArray (PrimState m) a)
mutableArrayFromListN n xs = do
  marr <- newPrimArray n
  fillMutableArrayWithListListN marr 0 n xs
  pure marr
fillMutableArrayWithListListN :: PrimMonad m => MutableArray (PrimState m) a -> Int -> Int -> [a] -> m ()

While inverse is not possible.

I cannot recall any instance where I wanted to fill an existing array with elements from a list.

I did provide a good use case: it would allow one to use multiple lists to fill a single array, which can be a useful feature for parallelization.

Here is a more important use case. With mutableArrayFromListN you do not have control of whether you want a pinned, pinned aligned or unpinned array. So, do you really want to provide three variants of this function?

I am also not aware of any existing library that offers such a function.

This is definitely not a good argument that this is not a useful function. There are plenty of useful functions that could be added to vector. Such fill function is one of them. There is one for streams: fill, which is sufficient for internal use, but regular users would probably prefer to work on lists rather than streams.

meooow25 commented 2 months ago

I did provide a good use case: it would allow one to use multiple lists to fill a single array, which can be a useful feature for parallelization.

I cannot judge the utility of this since I don't have an experience of being in such a situation.

Here is a more important use case. With mutableArrayFromListN you do not have control of whether you want a pinned, pinned aligned or unpinned array. So, do you really want to provide three variants of this function?

  1. This only applies to MutableByteArray#. MutableArray# and SmallMutableArray# don't have to deal with this.
  2. I see many functions creating MutableByteArray# using newByteArray# where any of the 3 variants could be used. So this library seems to have already decided to use unpinned arrays by default and not provide 3 variants for every such function.

This is definitely not a good argument that this is not a useful function.

It is not an argument that it is not a useful function, it is simply pointing out that existing libraries seem to not have acknowledged the need for such a function.

There is one for streams: fill, which is sufficient for internal use...

As far as I can tell, the only internal usage of fill is to mapM a mutable vector in place using a stream reading from the same vector. While I can see why this is useful, this task cannot be done by a list and should not be likened to it.


Anyway, though I personally don't see the need for this, it is up to the maintainers to decide.

lehins commented 2 months ago

FYI. I totally support adding generate and generateM

I am also not against adding mutableArrayFromListN. I can see how it can be useful too. All I am trying to say is that it can be implemented in terms of fillMutableArrayWithListN, so it might make sense to export such function as well. But as you very well pointed out, it is up to maintainers to decide.

meooow25 commented 2 months ago

@andrewthad what are your thoughts?