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

[#27] Introduce Terminating constraint for preventing hanging showMemory #46

Closed
1 change: 1 addition & 0 deletions membrain.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ common common-options
-fhide-source-paths
-Wmissing-export-lists
-Wpartial-fields
-freduction-depth=250
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I can't find documentation for this. (I'm looking into https://downloads.haskell.org/~ghc/latest/docs/html/users_guide/flags.html )
Could you please explain why this is needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

I want to add from my side that adding custom and rare flags (that apparently doesn't have documentation) might be a stopper for merging. This constant might work for natural 7000. But GHC might require a much higher constant for bigger naturals. This can slow down compilation significantly for the membrain users or produce difficult errors. The fact that this constant needs to patched to make the library work looks like an undesirable property to me.

Copy link
Author

Choose a reason for hiding this comment

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

Hi, answers inline.

Sorry, I can't find documentation for this

Yes, flag not being documented is in fact a ghc bug: https://gitlab.haskell.org/ghc/ghc/issues/15756

GHC manpage seems to have this documented though:
https://manpages.debian.org/stretch/ghc/haskell-compiler.1.en.html#Language_options

This flag controls how many steps GHC is allowed to take for simplifying a single type expression.

Could you please explain why this is needed?

In order to understand whether large Nats are only dividable by 5 or 2, but not by any other number, we need to continuously divide them by 5 and 2 and see if we end up with 1.
This creates a type expression like (((((7000 Div 5) Div 5) Div 5) Div 5)... for ghc to simplify and ghc simply has a limit on number of reductions it wants to perform.

For Yobibyte, currently the largest unit, this reduction size is exactly 250, hence the -freduction-depth=250 flag.

if impl(ghc >= 8.8.1)
ghc-options: -Wmissing-deriving-strategies
-Werror=missing-deriving-strategies
Expand Down
16 changes: 14 additions & 2 deletions src/Membrain/Memory.hs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
{-# OPTIONS_GHC -fno-warn-redundant-constraints #-}
{-# LANGUAGE AllowAmbiguousTypes #-}
{-# LANGUAGE CPP #-}
{-# LANGUAGE DataKinds #-}
{-# LANGUAGE DerivingStrategies #-}
{-# LANGUAGE ExistentialQuantification #-}
Expand Down Expand Up @@ -56,6 +58,10 @@ import Numeric.Natural (Natural)

import Membrain.Units (KnownUnitSymbol, unitSymbol)

#if ( __GLASGOW_HASKELL__ >= 804 )
import Membrain.Units (Terminating)
#endif

import qualified Prelude


Expand Down Expand Up @@ -104,6 +110,12 @@ instance Monoid (Memory (mem :: Nat)) where
mconcat = foldl' (<>) mempty
{-# INLINE mconcat #-}

#if ( __GLASGOW_HASKELL__ >= 804 )
type KnownMem mem = (KnownNat mem, KnownUnitSymbol mem, Terminating mem)
#else
type KnownMem mem = (KnownNat mem, KnownUnitSymbol mem)
#endif

{- |
This 'showMemory' function shows a 'Memory' value as 'Double' along with the
measure unit suffix. It shows 'Memory' losslessly while used with standardized
Expand All @@ -119,7 +131,7 @@ different forms of units then the 'show' function for 'Memory' hangs.
>>> showMemory (Memory 22 :: Memory Byte)
"2.75B"
-}
showMemory :: forall mem . (KnownNat mem, KnownUnitSymbol mem) => Memory mem -> String
showMemory :: forall mem . (KnownMem mem) => Memory mem -> String
showMemory (Memory m) = showFrac m (nat @mem) ++ unitSymbol @mem
where
showFrac :: Natural -> Natural -> String
Expand Down Expand Up @@ -308,7 +320,7 @@ collections, or when 'Memory' of non-specified unit can be returned.

-- | Existential data type for 'Memory's.
data AnyMemory
= forall (mem :: Nat) . (KnownNat mem, KnownUnitSymbol mem)
= forall (mem :: Nat) . (KnownMem mem)
=> MkAnyMemory (Memory mem)

instance Show AnyMemory where
Expand Down
50 changes: 49 additions & 1 deletion src/Membrain/Units.hs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
{-# LANGUAGE MagicHash #-}
{-# LANGUAGE TypeFamilyDependencies #-}
{-# LANGUAGE TypeOperators #-}
{-# LANGUAGE UndecidableInstances #-}
#if ( __GLASGOW_HASKELL__ >= 806 )
{-# LANGUAGE NoStarIsType #-}
#endif
Expand Down Expand Up @@ -58,11 +59,20 @@ module Membrain.Units
, UnitSymbol
, KnownUnitSymbol
, unitSymbol

#if ( __GLASGOW_HASKELL__ >= 804 )
-- * Type family for unit safety
, Terminating
#endif
) where

import Data.Kind (Constraint)
import GHC.Exts (Proxy#, proxy#)
import GHC.TypeLits (KnownSymbol, Symbol, symbolVal')
import GHC.TypeLits (ErrorMessage (..), KnownSymbol, Symbol, TypeError, symbolVal')
import GHC.TypeNats (type (*), Nat)
#if ( __GLASGOW_HASKELL__ >= 804 )
import GHC.TypeNats (Div, Mod)
#endif


type Bit = 1
Expand Down Expand Up @@ -140,3 +150,41 @@ only with @-XTypeApplications@.
unitSymbol :: forall (mem :: Nat) . KnownUnitSymbol mem => String
unitSymbol = symbolVal' (proxy# :: Proxy# (UnitSymbol mem))
{-# INLINE unitSymbol #-}

#if ( __GLASGOW_HASKELL__ >= 804 )

-- | Decides whether given decimal is a terminating decimal or not.
type family Terminating (val :: Nat) :: Constraint where
Terminating v = TerminatingOrig v v

-- | Decides whether given decimal is a terminating decimal or not but preserves
-- the original number. It's used for generating the error message in @Terminating2@.
type family TerminatingOrig (original :: Nat) (val :: Nat) :: Constraint where
TerminatingOrig _ 1 = ()
TerminatingOrig original v = Terminating5 original (v `Mod` 5) v

-- | Decides whether given decimal is divisible by 5 or not.
-- If it is divisible, divides it and sends it back to @TerminatingOrig@ for further reduction.
-- If it is not, sends it to @Terminating2@ to see if it's in the form of 2^x.
type family Terminating5 (original :: Nat) (mod_result :: Nat) (val :: Nat) :: Constraint where
Terminating5 original 0 v = TerminatingOrig original (v `Div` 5)
Terminating5 original _ v = Terminating2 original (v `Mod` 2) v

-- | Decides whether given decimal is divisible by 2 or not.
-- If it is divisible, divides it and sends it back to @TerminatingOrig@ for further reduction.
-- If it is not, this is an error since;
-- The value is not 1
-- The value is not divisible by 2 or 5.
--
-- Conditions above means that this is not a "terminating decimal":
-- https://en.wikipedia.org/wiki/Repeating_decimal
type family Terminating2 (original :: Nat) (mod_result :: Nat) (val :: Nat) :: Constraint where
Terminating2 original 0 v = TerminatingOrig original (v `Div` 2)
Terminating2 original _ mul =
TypeError ('Text "Value "
':<>: 'ShowType original
':<>: 'Text " should only multipliers of 2 and 5"
':<>: 'Text " (ie. should be in form 2^x.5^y)"
yigitozkavci marked this conversation as resolved.
Show resolved Hide resolved
':$$: 'Text "but it has the multiplier " ':<>: 'ShowType mul
':$$: 'Text "See showMemory function for why this is needed.")
#endif
1 change: 1 addition & 0 deletions test/Doctest.hs
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ main = do
: "-XDerivingStrategies"
: "-XGeneralizedNewtypeDeriving"
: "-XDeriveGeneric"
: "-freduction-depth=250"
: sourceFiles