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

fromList for mutable arrays #413

Open
meooow25 opened this issue May 1, 2024 · 7 comments
Open

fromList for mutable arrays #413

meooow25 opened this issue May 1, 2024 · 7 comments

Comments

@meooow25
Copy link
Contributor

meooow25 commented May 1, 2024

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
Copy link
Collaborator

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
Copy link
Contributor

lehins commented May 1, 2024

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
Copy link
Contributor Author

meooow25 commented May 1, 2024

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
Copy link
Contributor

lehins commented May 1, 2024

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
Copy link
Contributor Author

meooow25 commented May 2, 2024

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
Copy link
Contributor

lehins commented May 2, 2024

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
Copy link
Contributor Author

meooow25 commented May 3, 2024

@andrewthad what are your thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants