Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion dataframe-core/src-internal/DataFrame/Internal/AggKernel.hs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ varScatter takeSqrt g nGroups v = runST $ do
| otherwise = do
c <- VUM.unsafeRead cnt k
mm <- VUM.unsafeRead m2 k
let var = if c < 2 then 0 else mm / fromIntegral (c - 1)
-- NaN at n = 1, matching computeVariance
let var = if c < 2 then 0 / 0 else mm / fromIntegral (c - 1)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmmm. @daikonradish does it make sense to stick a NaN here or fail instead. I think the right thing is to make it optional but the ergonomics of dealing with Maybe make me question putting it in the happy path.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's the same code the groupBy interpreter path uses... so Ig throwing there means one singleton group kills the whole aggregation (and the kernels would have to match)? NaN keeps the singleton visible without killing the query, and matches pandas/numpy (sample variance of n=1 -> NaN). Can simply switch all three to throw... (if u want it to fail loud)

VUM.unsafeWrite out k (if takeSqrt then sqrt var else var)
fin (k + 1)
fin 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,8 @@ varPar takeSqrt vis offs nGroups v caps bounds = do
| otherwise = do
c <- VUM.unsafeRead cnt k
mm <- VUM.unsafeRead m2 k
let var = if c < 2 then 0 else mm / fromIntegral (c - 1)
-- NaN at n = 1, matching computeVariance
let var = if c < 2 then 0 / 0 else mm / fromIntegral (c - 1)
VUM.unsafeWrite out k (if takeSqrt then sqrt var else var)
fin (k + 1)
fin 0
Expand Down
22 changes: 22 additions & 0 deletions dataframe-core/src-internal/DataFrame/Internal/Column.hs
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,28 @@ columnBitmap (UnboxedColumn bm _) = bm
columnBitmap (PackedText bm _) = bm
columnBitmap (MergedColumn _ _) = Nothing

{- | Drops the null values in a nullable column: those slots hold a sentinel,
not a value. Identity on columns without a bitmap. 'VG.ifilter' inspects only
the index, so boxed error thunks at null slots are never forced.
-}
dropNulls :: Column -> Column
dropNulls (BoxedColumn (Just bm) xs) =
BoxedColumn Nothing (VG.ifilter (\i _ -> bitmapTestBit bm i) xs)
dropNulls (UnboxedColumn (Just bm) xs) =
UnboxedColumn Nothing (VG.ifilter (\i _ -> bitmapTestBit bm i) xs)
dropNulls c@(PackedText (Just _) _) = dropNulls (materializePacked c)
dropNulls c = c
{-# INLINE dropNulls #-}

{- | 'dropNulls', unless the view type @a@ is @Maybe@-headed: a @Maybe@-typed
view encodes the nulls as values, so the column passes through untouched.
-}
dropNullsExceptMaybe :: forall a. (Typeable a) => Column -> Column

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is confusing. Why doesn't drop nulls handle this case. That would be a good thing to put in the comment. Let me read the calm site.

dropNullsExceptMaybe c = case typeRep @a of
App m _ | Just HRefl <- eqTypeRep m (typeRep @Maybe) -> c
_ -> dropNulls c
{-# INLINE dropNullsExceptMaybe #-}

{- | Decode a 'PackedText' into a @BoxedColumn Text@ (bit-identical to
materializing at freeze). Identity on every other column.
-}
Expand Down
11 changes: 5 additions & 6 deletions dataframe-learn/tests-internal/Learn/EdgeCases.hs
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,13 @@ testVarianceConstant = TestCase $ do
let v = variance' (VU.replicate 100 (7.0 :: Double))
assertEqual "variance of constant column is 0" 0 v

{- Variance of fewer than two samples is defined to be 0 (computeVariance guard),
not NaN from a /0. -}
{- Sample variance of one observation is undefined: NaN, so a singleton
group can never look as tight as a genuinely constant column. -}
testVarianceSingleton :: Test
testVarianceSingleton = TestCase $ do
assertEqual
"variance of one sample is 0"
0
(variance' (VU.fromList [3.5 :: Double]))
assertBool
"variance of one sample is NaN"
(isNaN (variance' (VU.fromList [3.5 :: Double])))

{- Correlation of a perfectly linear pair is exactly +1 (and -1 reversed),
computed stably. y = 2x+1 over a spread of x. -}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ varianceStep (VarAcc !n !meanVal !m2) !x =

computeVariance :: VarAcc -> Double
computeVariance (VarAcc !n _ !m2)
| n < 2 = 0 -- or error "variance of <2 samples"
| n == 0 = throw $ EmptyDataSetException "variance"
-- undefined at n = 1: NaN, not 0
| n < 2 = 0 / 0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Throw here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think Variance' is also what the groupBy path calls (variance = Agg (CollectAgg "variance" variance') in Functions.hs), so a throw here kinda takes down a whole aggregation when any group is a singleton, and the scatter kernels would need to throw too to stay in sync. It's left as NaN for now, same question as the kernel thread.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Cool. Maybe to make this PR easier to reason about please add specific examples or behaviour changes in the PR description. Sort of like how a front end engineer adds screenshots to their PRs. A clear before and after of failure modes and their new fixes - they could even be copies of the test examples but they should be something like:

Before:

$ ./scripts/repl.sh
ghci> :script dataframe.ghci
dataframe> df <- D.readCsv "./data/housing.csv"
dataframe> -- the operation

Then after would paste a similar thing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added before/after examples to the description.

| otherwise = m2 / fromIntegral (n - 1)
{-# INLINE computeVariance #-}

Expand Down
124 changes: 77 additions & 47 deletions dataframe-operations/src/DataFrame/Operations/Statistics.hs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ mean (Col name) df = case _getColumnAsDouble name df of
Nothing -> error "[INTERNAL ERROR] Column is non-numeric"
mean expr df = case interpret df expr of
Left e -> throw e
Right (TColumn col) -> case toUnboxedVector @a col of
Right (TColumn col) -> case toUnboxedVector @a (dropNulls col) of
Left e -> throw e
Right xs -> mean' xs

Expand All @@ -136,12 +136,15 @@ meanMaybe expr df = case interpret @(Maybe a) df expr of
-- | Calculates the median of a given column as a standalone value.
median ::
forall a. (Columnable a, Real a, VU.Unbox a) => Expr a -> DataFrame -> Double
median (Col name) df = case columnAsUnboxedVector (Col @a name) df of
Right xs -> median' xs
Left e -> throw e
median (Col name) df = case getColumn name df of
Just col -> case toUnboxedVector @a (dropNulls col) of
Right xs -> median' xs
Left e -> throw e
Nothing ->
throw $ ColumnsNotFoundException [name] "median" (M.keys $ columnIndices df)
median expr df = case interpret df expr of
Left e -> throw e
Right (TColumn col) -> case toUnboxedVector @a col of
Right (TColumn col) -> case toUnboxedVector @a (dropNulls col) of
Left e -> throw e
Right xs -> median' xs

Expand All @@ -161,49 +164,63 @@ medianMaybe expr df = case interpret @(Maybe a) df expr of
percentile ::
forall a.
(Columnable a, Real a, VU.Unbox a) => Int -> Expr a -> DataFrame -> Double
percentile n (Col name) df = case columnAsUnboxedVector (Col @a name) df of
Right xs -> percentile' n xs
Left e -> throw e
percentile n (Col name) df = case getColumn name df of
Just col -> case toUnboxedVector @a (dropNulls col) of
Right xs -> percentile' n xs
Left e -> throw e
Nothing ->
throw $ ColumnsNotFoundException [name] "percentile" (M.keys $ columnIndices df)
percentile n expr df = case interpret df expr of
Left e -> throw e
Right (TColumn col) -> case toUnboxedVector @a col of
Right (TColumn col) -> case toUnboxedVector @a (dropNulls col) of
Left e -> throw e
Right xs -> percentile' n xs

-- | Calculates the nth percentile of a given column as a standalone value.
genericPercentile ::
forall a.
(Columnable a, Ord a) => Int -> Expr a -> DataFrame -> a
genericPercentile n (Col name) df = case columnAsVector (Col @a name) df of
Right xs -> percentileOrd' n xs
Left e -> throw e
genericPercentile n (Col name) df = case getColumn name df of
Just col -> case toVector @a (dropNullsExceptMaybe @a col) of
Right xs -> percentileOrd' n xs
Left e -> throw e
Nothing ->
throw $
ColumnsNotFoundException [name] "genericPercentile" (M.keys $ columnIndices df)
genericPercentile n expr df = case interpret df expr of
Left e -> throw e
Right (TColumn col) -> case toVector @a col of
Right (TColumn col) -> case toVector @a (dropNullsExceptMaybe @a col) of
Left e -> throw e
Right xs -> percentileOrd' n xs

-- | Calculates the standard deviation of a given column as a standalone value.
standardDeviation ::
forall a. (Columnable a, Real a, VU.Unbox a) => Expr a -> DataFrame -> Double
standardDeviation (Col name) df = case columnAsUnboxedVector (Col @a name) df of
Right xs -> (sqrt . variance') xs
Left e -> throw e
standardDeviation (Col name) df = case getColumn name df of
Just col -> case toUnboxedVector @a (dropNulls col) of
Right xs -> (sqrt . variance') xs
Left e -> throw e
Nothing ->
throw $
ColumnsNotFoundException [name] "standardDeviation" (M.keys $ columnIndices df)
standardDeviation expr df = case interpret df expr of
Left e -> throw e
Right (TColumn col) -> case toUnboxedVector @a col of
Right (TColumn col) -> case toUnboxedVector @a (dropNulls col) of
Left e -> throw e
Right xs -> (sqrt . variance') xs

-- | Calculates the skewness of a given column as a standalone value.
skewness ::
forall a. (Columnable a, Real a, VU.Unbox a) => Expr a -> DataFrame -> Double
skewness (Col name) df = case columnAsUnboxedVector (Col @a name) df of
Right xs -> skewness' xs
Left e -> throw e
skewness (Col name) df = case getColumn name df of
Just col -> case toUnboxedVector @a (dropNulls col) of
Right xs -> skewness' xs
Left e -> throw e
Nothing ->
throw $ ColumnsNotFoundException [name] "skewness" (M.keys $ columnIndices df)
skewness expr df = case interpret df expr of
Left e -> throw e
Right (TColumn col) -> case toUnboxedVector @a col of
Right (TColumn col) -> case toUnboxedVector @a (dropNulls col) of
Left e -> throw e
Right xs -> skewness' xs

Expand All @@ -215,42 +232,51 @@ variance (Col name) df = case _getColumnAsDouble name df of
Nothing -> error "[INTERNAL ERROR] Column is non-numeric"
variance expr df = case interpret df expr of
Left e -> throw e
Right (TColumn col) -> case toUnboxedVector @a col of
Right (TColumn col) -> case toUnboxedVector @a (dropNulls col) of
Left e -> throw e
Right xs -> variance' xs

-- | Calculates the inter-quartile range of a given column as a standalone value.
interQuartileRange ::
forall a. (Columnable a, Real a, VU.Unbox a) => Expr a -> DataFrame -> Double
interQuartileRange (Col name) df = case columnAsUnboxedVector (Col @a name) df of
Right xs -> interQuartileRange' xs
Left e -> throw e
interQuartileRange (Col name) df = case getColumn name df of
Just col -> case toUnboxedVector @a (dropNulls col) of
Right xs -> interQuartileRange' xs
Left e -> throw e
Nothing ->
throw $
ColumnsNotFoundException [name] "interQuartileRange" (M.keys $ columnIndices df)
interQuartileRange expr df = case interpret df expr of
Left e -> throw e
Right (TColumn col) -> case toUnboxedVector @a col of
Right (TColumn col) -> case toUnboxedVector @a (dropNulls col) of
Left e -> throw e
Right xs -> interQuartileRange' xs

-- | Calculates the Pearson's correlation coefficient between two given columns as a standalone value.
{- | Calculates the Pearson's correlation coefficient between two given columns as a standalone value.
Pairs with a null in either column are dropped.
-}
correlation :: T.Text -> T.Text -> DataFrame -> Maybe Double
correlation first second df = do
f <- _getColumnAsDouble first df
s <- _getColumnAsDouble second df
let df' = filterJust first (filterJust second df)
f <- _getColumnAsDouble first df'
s <- _getColumnAsDouble second df'
correlation' f s

_getColumnAsDouble :: T.Text -> DataFrame -> Maybe (VU.Vector Double)
_getColumnAsDouble name df = case getColumn name df of
Just (UnboxedColumn _ (f :: VU.Vector a)) -> case testEquality (typeRep @a) (typeRep @Double) of
Just Refl -> Just f
Nothing -> case sIntegral @a of
STrue -> Just (VU.map fromIntegral f)
SFalse -> case sFloating @a of
STrue -> Just (VU.map realToFrac f)
SFalse -> Nothing
Just col -> case dropNulls col of
UnboxedColumn _ (f :: VU.Vector a) ->
case testEquality (typeRep @a) (typeRep @Double) of
Just Refl -> Just f
Nothing -> case sIntegral @a of
STrue -> Just (VU.map fromIntegral f)
SFalse -> case sFloating @a of
STrue -> Just (VU.map realToFrac f)
SFalse -> Nothing
_ -> Nothing
Nothing ->
throw $
ColumnsNotFoundException [name] "_getColumnAsDouble" (M.keys $ columnIndices df)
_ -> Nothing
{-# INLINE _getColumnAsDouble #-}

optionalToDoubleVector :: (Real a) => V.Vector (Maybe a) -> VU.Vector Double
Expand All @@ -265,17 +291,18 @@ sum ::
forall a. (Columnable a, Num a) => Expr a -> DataFrame -> a
sum (Col name) df = case getColumn name df of
Nothing -> throw $ ColumnsNotFoundException [name] "sum" (M.keys $ columnIndices df)
Just ((UnboxedColumn _ (column :: VU.Vector a'))) -> case testEquality (typeRep @a') (typeRep @a) of
Just Refl -> VG.sum column
Nothing -> 0
Just ((BoxedColumn _ (column :: V.Vector a'))) -> case testEquality (typeRep @a') (typeRep @a) of
Just Refl -> VG.sum column
Nothing -> 0
Just (PackedText _ _) -> 0
Just (MergedColumn _ _) -> 0 -- matches the old eager These column (type never Num)
Just c -> case dropNulls c of
UnboxedColumn _ (column :: VU.Vector a') -> case testEquality (typeRep @a') (typeRep @a) of
Just Refl -> VG.sum column
Nothing -> 0
BoxedColumn _ (column :: V.Vector a') -> case testEquality (typeRep @a') (typeRep @a) of
Just Refl -> VG.sum column
Nothing -> 0
PackedText _ _ -> 0
MergedColumn _ _ -> 0 -- never numeric
sum expr df = case interpret df expr of
Left e -> throw e
Right (TColumn xs) -> case toVector @a @V.Vector xs of
Right (TColumn xs) -> case toVector @a @V.Vector (dropNulls xs) of
Left e -> throw e
Right xs' -> VG.sum xs'

Expand Down Expand Up @@ -416,7 +443,10 @@ summarize df =

-- | Round a @Double@ to Specified Precision
roundTo :: Int -> Double -> Double
roundTo n x = fromInteger (round $ x * 10 ^ n) / 10.0 ^^ n
roundTo n x
-- round on NaN is garbage
| isNaN x = x
| otherwise = fromInteger (round $ x * 10 ^ n) / 10.0 ^^ n

toPct2dp :: Double -> String
toPct2dp x
Expand Down
1 change: 0 additions & 1 deletion dataframe.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,6 @@ test-suite tests
Internal.DictEncode,
Internal.Markdown,
Internal.PackedText,
Internal.Parsing,
PackedTextMigration,
PrettyPrint,
Learn.Denotation,
Expand Down
Loading
Loading