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

Constructors for immutable unboxed tuple vectors are not exported #504

Closed
ruifengx opened this issue Oct 5, 2024 · 6 comments
Closed

Constructors for immutable unboxed tuple vectors are not exported #504

ruifengx opened this issue Oct 5, 2024 · 6 comments

Comments

@ruifengx
Copy link

ruifengx commented Oct 5, 2024

See the following export list for Data.Vector.Unboxed:

module Data.Vector.Unboxed (
-- * Unboxed vectors
Vector(V_UnboxAs, V_UnboxViaPrim), MVector(..), Unbox,

All the constructors for the MVector family are exported, which includes constructors for mutable unboxed tuple vectors (e.g., MV_2). However, for immutable unboxed tuple vectors, the constructors are private.

Currently, to construct an unboxed tuple vector from the two underlying parallel vector without copying, one must go through MVector and unsafeFreeze, which seems a bit weird to me.

import Data.Vector.Unboxed qualified as U
pairUnboxed :: U.Vector a -> U.Vector b -> U.Vector (a, b)
pairUnboxed xs ys = do
    xs' <- unsafeThaw xs
    ys' <- unsafeThaw ys
    let len = U.length xs
    unsafeFreeze (MV_2 len xs' ys')
@toyboot4e
Copy link
Contributor

toyboot4e commented Oct 5, 2024

Related issues: #49, #492. Unboxed vector's constructor seem like exposed from Data.Vector.Unboxed.Base. (I'm not sure about the details).

@ruifengx
Copy link
Author

ruifengx commented Oct 5, 2024

Indeed, this seems like a duplicate of #492, then. I guess the only new point is that not exporting V_2 does not actually prevent the user from achieving the same functionality, just with more boilerplate. Maybe I should close this one in favour of #492?

@Shimuuar
Copy link
Contributor

Shimuuar commented Oct 5, 2024

In fact zip/unzip, zip3/unzip3 etc exported from D.V.Unboxed and will grant access to underlying vectors. This is just poorly documented. Basically this is a documentation problem

@ruifengx
Copy link
Author

ruifengx commented Oct 5, 2024

Thanks! Somehow I did not think this way. I must have thought those functions are just aliases to the ones in D.V.Generic (for fusion, of course), and I did not find their implementations on Hackage. Checking their implementations in the included file unboxed-tuple-instances, it is indeed just what I was looking for:

-- | /O(1)/ Zip 2 vectors.
zip :: (Unbox a, Unbox b) => Vector a -> Vector b -> Vector (a, b)
{-# INLINE_FUSED zip #-}
zip as bs = V_2 len (unsafeSlice 0 len as) (unsafeSlice 0 len bs)
where len = length as `delayed_min` length bs
{-# RULES "stream/zip [Vector.Unboxed]" forall as bs .
G.stream (zip as bs) = Bundle.zipWith (,) (G.stream as)
(G.stream bs) #-}

Actually, in their documentation they are clearly marked O(1). Maybe it can be improved in discoverability by mentioning this fact at the very top where representations of unboxed vectors are discussed.

@Shimuuar
Copy link
Contributor

Shimuuar commented Oct 6, 2024

they are clearly marked O(1)

That's simple puzzle but even simple puzzles don't make good documentation

@Shimuuar
Copy link
Contributor

I hope that #505 is good enough so I'm closing it. @ruifengx ping if you disagree

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