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

Double arithmetic? #166

Closed
michalrus opened this issue Jun 1, 2018 · 29 comments
Closed

Double arithmetic? #166

michalrus opened this issue Jun 1, 2018 · 29 comments

Comments

@michalrus
Copy link

I know it’s deliberately left out, but we have quite a few of these in config files:

, cookieMaxAge = 1209600.0 -- 14 * 24 * 60 * 60

As sometimes sub-second NominalDiffTimes are needed:

instance Dhall.Interpret NominalDiffTime where
  autoWith _ = fromRational . toRational <$> Dhall.double

What would be the best way to go here?

@Gabriella439
Copy link
Contributor

How about adding a Natural/toDouble built-in? Then you could do:

cookieMaxAge = Natural/toDouble (14 * 24 * 60 * 60)

... or Integer/toDouble (since Natural/toInteger exists) or both

@ocharles
Copy link
Member

ocharles commented Jun 1, 2018 via email

@Gabriella439
Copy link
Contributor

Yeah, I'm also leaning towards only providing Integer/toDouble to keep the built-ins as orthogonal as possible

@michalrus
Copy link
Author

Integer/toDouble would be wonderful! =)

Gabriella439 added a commit that referenced this issue Jun 7, 2018
@Gabriella439
Copy link
Contributor

The proposed change to the standard is here: #168

@michalrus
Copy link
Author

@Gabriel439 one more thought: this won’t allow Double calculations like 0.1 * 0.5, or this one (for which I have a use case):

{
  partOfInitialX = 0.00333333333333333333 -- = 5 / 1500
}

@Gabriella439
Copy link
Contributor

@michalrus: That's correct. The proposed change still doesn't permit double arithmetic

@michalrus
Copy link
Author

Yes, yes, I totally understand that from the diff and this discussion above.

But, now, after going through our Dhall files, I’m thinking that m-m-maaaybe being able to say 5/1500 instead of 0.00333333333333333333 -- = 5 / 1500 could also be seen as desirable. I don’t know. 🤷‍♂️

@michalrus
Copy link
Author

I should have read all of the files when posting this issue, sorry. 😿

@Gabriella439
Copy link
Contributor

@michalrus: No worries :)

I think I'll postpone adding Double arithmetic for now because that opens up a whole can of worms (like how Dhall should handle division by 0, for example)

Gabriella439 added a commit that referenced this issue Jun 12, 2018
@statusfailed
Copy link

I've ended up with another use-case for Double arithmetic- I wanted to have some user-supplied templates that are essentially "fragments" of SVG, and know how to draw themselves at supplied coordinates + dimensions.

Basically, the user-supplied Dhall expression is something like this:

\(dimensions: { x: Double, y: Double, width: Double, height: Double }) -> ''
<rect cx="${Double/show (x + w/4)}"
      cy="${Double/show (y + h/4)"
      width="${Double/show (width/2)}"
      height="${Double/show (height/2)}"
/>
''

Would there be an issue with adding division only for Doubles, and just allowing "Infinity" values when division by zero occurs? I see that division for other types might be a problem (because of exceptions) though.

@FintanH
Copy link
Collaborator

FintanH commented Jan 8, 2019

@statusfailed What if you made a new data type that represented ratios like { numerator : Natural, denominator : Natural } and then could define division and the likes for that type? I believe @sellout has described something similar to me before too.

@sellout
Copy link
Collaborator

sellout commented Jan 8, 2019

@statusfailed I don’t know that it’ll provide what you want, but as @FintanH alluded to, I do have a nascent “dhall-numerics” library I’ve been working on. I can publish it later today, and you can take a look to see what’s there.

@statusfailed
Copy link

@FintanH I think I'd still need a function to get the decimal representation for my final output, right? Can this be done within Dhall?

To solve the immediate problem I might just add Double/div in this way

@statusfailed
Copy link

@sellout sure, I'll have a look, thanks!

@FintanH
Copy link
Collaborator

FintanH commented Jan 8, 2019

So the idea would be that you delay the computation until you interpret into your final language, e.g. Haskell, and then it will translate your Ratio into Double. The plus side of this is that you won't have the potential of precision errors until the very last moment :)

@Gabriella439
Copy link
Contributor

@statusfailed: Yeah, I think you will probably want to add built-in Double operators for your use case

@FintanH: My understanding is that implementing the Double conversion logic in Haskell wouldn't work for this use because they want to use the resulting Double to template a string literal within Dhall

@FintanH
Copy link
Collaborator

FintanH commented Jan 8, 2019

My understanding is that implementing the Double conversion logic in Haskell wouldn't work for this use because they want to use the resulting Double to template a string literal within Dhall

Unless you can delay the show to Haskell as well? 🤔 But maybe I didn't understand the use case properly :)

@statusfailed
Copy link

@FintanH @Gabriel439 I threw together a little example of how this can be done here based on the Wiki guide.

Hopefully this helps anyone else who needs to do the same. Thanks everyone for your help!

@Gabriella439
Copy link
Contributor

@statusfailed: Awesome! Thank you 🙂

@f-f
Copy link
Member

f-f commented Jan 9, 2019

Probably worth adding to the Wiki guide as a working example?

@statusfailed
Copy link

Hmm... seems like I've hit another problem: I'm not able to use (in Haskell-land) functions defined in terms of my newly-added Double arithmetic functions. For example, when I define a dhall file like this:

\(x : Double) -> \(y : Double) -> (Double/add x y)

I get this error in my Haskell program when trying to use it with Dhall.inputWithSettings doubleArithmeticSettings Dhall.auto text:

*** Exception: Interpret: You cannot decode a function if it does not have the correct type

Whereas if I only use "existing" Double functions (like the below Dhall function), everything works:

\(x : Double) -> \(y : Double) -> (Double/show x)

Is there a way I can solve this?

@ocharles
Copy link
Member

ocharles commented Jan 9, 2019

Can you provide a full example?

statusfailed added a commit to statusfailed/double-dhall that referenced this issue Jan 9, 2019
From dhall-lang issue #166 here:
dhall-lang/dhall-lang#166

Using "broken.dhall" will result in this exception:

    *** Exception: Interpret: You cannot decode a function if it does not have the correct type

Using "working.dhall" will display "1.0".
@statusfailed
Copy link

I've created a full example on the branch example/cannot-decode-function of the double-dhall repo.

There are two Dhall files added. broken.dhall yields the below exception when run with cat broken.dhall | cabal run:

double-dhall: Interpret: You cannot decode a function if it does not have the correct type
CallStack (from HasCallStack):
  error, called at src/Dhall.hs:803:24 in dhall-1.19.1-CXfxNxFUhESEV34XV95wYt:Dhall

And working.dhall prints 1.0 as expected.

FWIW, I am using Nix, so running this with version 1.19.1 of Dhall - not sure if this would make a difference. Let me know if there's anything else I can provide - cheers!

@Gabriella439
Copy link
Contributor

@statusfailed: I'll check this soon. Version 1.19.1 should be new enough since we haven't changed anything related to this since then

@Gabriella439
Copy link
Contributor

Gabriella439 commented Jan 11, 2019

@statusfailed: I found the problem. The issue is that the Interpret instance for functions isn't respecting your custom normalization setting:

instance (Inject a, Interpret b) => Interpret (a -> b) where
    autoWith opts = Type extractOut expectedOut
      where
        -- The `Dhall.Core.normalize` is ignoring your customizations
        extractOut e = Just (\i -> case extractIn (Dhall.Core.normalize (App e (embed i))) of
            Just o  -> o
            Nothing -> error "Interpret: You cannot decode a function if it does not have the correct type" )

        expectedOut = Pi "_" declared expectedIn

        InputType {..} = inject

        Type extractIn expectedIn = autoWith opts

This is why it works on built-in functions but not with your custom additions

I think fixing this properly requires adding a field to InterpretOptions to allow for a custom normalizer

Gabriella439 added a commit to dhall-lang/dhall-haskell that referenced this issue Jan 11, 2019
This fixes the problem found in dhall-lang/dhall-lang#166 (comment)

This ensures that the functions marshalled into Haskell can correctly take
advantage of user-supplied language extensions
@Gabriella439
Copy link
Contributor

Alright, I have a fix up here: dhall-lang/dhall-haskell#777

I was able to verify it works using the following change to your double-dhall repository:

diff --git a/Main.hs b/Main.hs
index abe5afc..16b6c06 100644
--- a/Main.hs
+++ b/Main.hs
@@ -60,12 +60,15 @@ doubleNormalizer =
 
 main :: IO ()
 main = do
-  text <- Data.Text.IO.getContents
   let context     = doubleEnrichedContext
       normalizer  = ReifiedNormalizer $ pure . doubleNormalizer
       addSettings = Lens.set Dhall.normalizer normalizer
                   . Lens.set Dhall.startingContext context
       inputSettings = addSettings Dhall.defaultInputSettings 
 
-  x <- Dhall.inputWithSettings inputSettings Dhall.auto text
-  Data.Text.IO.putStrLn x
+      interpretOptions =
+          Dhall.defaultInterpretOptions
+              { Dhall.inputNormalizer = normalizer }
+
+  f <- Dhall.inputWithSettings inputSettings (Dhall.autoWith interpretOptions) "Double/add" :: IO (Double -> Double -> Double)
+  print (f 1 2)

statusfailed added a commit to statusfailed/double-dhall that referenced this issue Jan 11, 2019
Updated Main.hs based on Gabriel439's code from
this thread dhall-lang/dhall-lang#166.

Also changed nix build to add these files:

* dhall-haskell.nix
* double-dhall.nix
* release.nix
* shell.nix

`dhall-haskell` points to the bleeding-edge commit
 (see this PR: dhall-lang/dhall-haskell#777)
which fixes the "You cannot decode a function..." problem.
@statusfailed
Copy link

Beautiful, works perfectly. I updated my branch on double-dhall to include your fix.

Thanks very much!

@Gabriella439
Copy link
Contributor

@statusfailed: You're welcome! Thank you for reporting the issue 🙂

Gabriella439 added a commit to dhall-lang/dhall-haskell that referenced this issue Jan 11, 2019
This fixes the problem found in dhall-lang/dhall-lang#166 (comment)

This ensures that the functions marshalled into Haskell can correctly take
advantage of user-supplied language extensions
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

7 participants