Rebased and fixed #347

[Shimuuar]

This is rebased #347 updated to work with current master.

• Makes indexM strict in vector by using basicIndexM
• Word trick is used to make single comparison for range checks instead of two for all range checks

[lehins]

That's great that you were able to confirm that this trick works!

[konsumlamm]

Sorry for not mentioning this earlier, but wouldn't it be more efficient to directly index in the Maybe monad, like this (which was also done in #347)?

v !? i | i `inRange` length v = basicUnsafeIndexM v i
       | otherwise            = Nothing

It's possible that the allocation of the Box is optimized away, but we probably shouldn't rely on that.

[gksato]

@konsumlamm

Sorry for not mentioning this earlier, but wouldn't it be more efficient to directly index in the Maybe monad, like this (which was also done in #347)?

v !? i | i `inRange` length v = basicUnsafeIndexM v i
       | otherwise            = Nothing

It's possible that the allocation of the Box is optimized away, but we probably shouldn't rely on that.

Note that

class (MVector (Mutable v) a) => Vector v a where
  ...
  basicUnsafeIndexM :: v a -> Int -> Box a
  ...

and

-- | /O(1)/ Unsafe indexing without bounds checking.
unsafeIndex :: Vector v a => v a -> Int -> a
{-# INLINE_FUSED unsafeIndex #-}
unsafeIndex v i = checkIndex Unsafe i (length v) $ unBox (basicUnsafeIndexM v i)

in favor of GeneralizedNewtypeDeriving.

[konsumlamm]

Huh, apparently this was changed a while ago in #335. Ignore my comment, then.

[gksato]

And I should also apologize that I should've mentioned this earlier, but I was afraid a function like factorial in what follows may get a poor optimization with this:

factorial :: HasCallStack => Int -> Mod1097
factorial n
  | n < 0 = error "factorial: pole of Gamma function"
  | n >= VU.length factorials = error "factorial: out of desired range"
  | otherwise = factorials VU.! n

factorials :: VU.Vector Mod1097
factorials = VU.scanl' (*) (M1097 1) $ VU.generate 300000 (M1097 . fromIntegral . (+1))

-- | Remainder ring mod 10^9+7.
newtype Mod1097 = M1097 Word32
  deriving newtype (Eq, Show, VGM.MVector VUM.MVector, VG.Vector VU.Vector, VUM.Unbox) 
newtype instance VU.Vector Mod1097 = UnboxMod1097 (VU.Vector Word32)
newtype instance VUM.MVector s Mod1097 = UnboxMMod1097 (VUM.MVector s Word32)

instance Num Mod1097 where ...

Anyway, I'll try this and check how it goes. If it goes wrong, I'll inform you all in a separate issue.

[Shimuuar]

@konsumlamm Yes it has been changed in order to allow GND/DerivingVia since GHC cannot coerce between m A and m B for polymorphic m even if A and B are coercible.

@gksato I think GHC is rather good at cutting away such allocations. But please do report your findings

[gksato]

Allocations? I meant comparisons.
I was afraid that GHC possibly cannot infer
fromIntegral n < (fromIntegral len :: Word)
even if given
n >= 0 && n < len.

[Shimuuar]

Ah! I wonder if GHC is able to remove duplicate checks even without such trick