Stream: contributing

Topic: Dec in repl

Derek Gustafson (Mar 04 2022 at 16:25):

I'm trying to get Decs to work in the repl. I've traced it back to repl_eval/src/eval.rs It looks like the function being passed to app.call_function needs to take a RocDec, which isn't included in the crate. Does this sound right? I want to make sure I'm not I'm not going about this in the wrong way before adding RocDec to the appropriate cargo file

Folkert de Vries (Mar 04 2022 at 16:36):

I think you can use deref_i128

Folkert de Vries (Mar 04 2022 at 16:36):

and then in the cli and wasm crates (that already import roc_std) do the actual conversion to RocDec

Folkert de Vries (Mar 04 2022 at 16:37):

repl_cli and repl_wasm, that is

Folkert de Vries (Mar 04 2022 at 16:38):

at least that's how we do it with strings and lists today

Derek Gustafson (Mar 04 2022 at 16:39):

I'll give that a try. Thanks!

Derek Gustafson (Mar 04 2022 at 18:23):

Now that I've dug deeper, what's needed in repl_eval is an Expr::Num, which takes a string representation of the number. I see 3 options:

1. Use deref_i128 and build the display logic into repl_eval. This will require leaking implementation details (how many decimal digits) of Dec into repl_eval
2. Add deref_dec(&self, addr: usize) -> &str to ReplAppMemory and add a function toStr(&self) -> &str to RocDec. This feels the cleanest to me.
3. Add deref_dec(&self, addr: usize) -> (i128, i32) to ReplAppMemory that returns the whole and decimal parts. Then add a function to repl_eval to display the pair. Note that there is already display logic for other numbers in repl_eval

Thoughts?

Folkert de Vries (Mar 04 2022 at 18:26):

that's surprising. What makes it different form list/str ?

Derek Gustafson (Mar 04 2022 at 18:29):

deref_str uses method 2.

Folkert de Vries (Mar 04 2022 at 18:31):

right, let's go with that then

Folkert de Vries (Mar 04 2022 at 18:32):

toStr is implemented in zig, but should be easy to port to rust

Derek Gustafson (Mar 04 2022 at 18:32):

I don't understand the List structure enough to be sure, but I think it basically uses 1 and leaks that it's implemented as an array.

Derek Gustafson (Mar 04 2022 at 18:33):

Should I leave the zig implementation for generating the llvm bytecode, or just convert the function to rust, and call that one?

Folkert de Vries (Mar 04 2022 at 18:33):

porting that code and putting it into roc_std should work

Folkert de Vries (Mar 04 2022 at 18:34):

the type signature there doesn't work though

Derek Gustafson (Mar 04 2022 at 18:34):

:+1:

Folkert de Vries (Mar 04 2022 at 18:34):

it has to be deref_dec(&self, addr: usize) -> i128, or at least something that is not a reference type

Derek Gustafson (Mar 04 2022 at 18:37):

Why not a &str?

Derek Gustafson (Mar 04 2022 at 18:39):

I guess the other possibility is deref_dec(&self, addr: usize) -> f64 and the print it as a f64

Folkert de Vries (Mar 04 2022 at 18:43):

lifetimes

Folkert de Vries (Mar 04 2022 at 18:43):

you cannot return a &str there, there is no way to allocate it

Folkert de Vries (Mar 04 2022 at 18:43):

it works in the case of deref_str because self contains the bytes

Folkert de Vries (Mar 04 2022 at 18:43):

but you cannot create a new string there and then return it as a &str

Derek Gustafson (Mar 04 2022 at 18:44):

Right

Folkert de Vries (Mar 04 2022 at 18:44):

f64 is a good option I think

Folkert de Vries (Mar 04 2022 at 18:44):

although

Folkert de Vries (Mar 04 2022 at 18:44):

maybe not

Folkert de Vries (Mar 04 2022 at 18:45):

we can represent 1/3 exactly I think right? and floats cannot

Folkert de Vries (Mar 04 2022 at 18:45):

so converting via float could introduce weird .000000001 problems

Folkert de Vries (Mar 04 2022 at 18:49):

you know what, mono is a dependency of repl_eval and it already imports roc_std, so adding it as a dependency has no effect whatsoever to repl_eval

Folkert de Vries (Mar 04 2022 at 18:49):

so making it deref_dec(...) -> RocDec should also be just fine

Derek Gustafson (Mar 04 2022 at 18:52):

And then call toStr to get the representation on the stack, and env.arena.alloc to get it on the heap, right?

Folkert de Vries (Mar 04 2022 at 18:57):

let's see, we want to do this right.

1) implement std::fmt::Display for RocStr
2) make a bumpalo::collections::String::with_capacity_in(max_num_of_digits)
3) use write!(bumaplo_string, "{}", roc_dec); it converts the RocDec to a string and pushes that result into the bumpalo string efficiently
4) there's probably some .into_bump_slice equivalent for strings, I'll look this up

Folkert de Vries (Mar 04 2022 at 18:59):

step 1 is only efficient if you write! the individual characters to the formatter, don't create a String in there

Folkert de Vries (Mar 04 2022 at 18:59):

the zig implementation does something similar

Derek Gustafson (Mar 04 2022 at 18:59):

Here's what's being called for the other numbers.
image.png

Folkert de Vries (Mar 04 2022 at 19:00):

oh that's not great

Folkert de Vries (Mar 04 2022 at 19:01):

you can use the later steps I described to create a bumpalo string directly

Derek Gustafson (Mar 04 2022 at 19:01):

Does this mean we can punt on steps 2-4?

Derek Gustafson (Mar 04 2022 at 19:02):

Or does it mean I should rewrite that function to use bumpalo?

Folkert de Vries (Mar 04 2022 at 19:04):

yeah, something like

fn number_literal_to_ast<T: std::fmt::Display>(arena: &Bump, num: T) -> Expr<'_> {
    let mut string = bumpalo::collections::String::with_capacity_in(64, arena);
    write!(string, "{}", num).unwrap();
    Expr::Num(string.into_bump_slice())
}

Folkert de Vries (Mar 04 2022 at 19:05):

that doesn't quite work yet though, I'll do some debugging

Derek Gustafson (Mar 04 2022 at 19:07):

I'll start by getting deref_dec and Display implemented

Folkert de Vries (Mar 04 2022 at 19:11):

got it

/// This is centralized in case we want to format it differently later,
/// e.g. adding underscores for large numbers
fn number_literal_to_ast<T: std::fmt::Display>(arena: &Bump, num: T) -> Expr<'_> {
    use std::fmt::Write;

    let mut string = bumpalo::collections::String::with_capacity_in(64, arena);
    write!(string, "{}", num).unwrap();
    Expr::Num(string.into_bump_str())
}

Brian Carroll (Mar 04 2022 at 19:17):

Oh yeah the original one allocates the string twice! Nice improvement.

Derek Gustafson (Mar 04 2022 at 19:18):

Do I need to worry about boxing/unboxing RocDec? Or can I just call this macro with RocDec?
image.png

Derek Gustafson (Mar 04 2022 at 19:19):

Well that's tiny

Folkert de Vries (Mar 04 2022 at 19:19):

I think the best approach is a default trait method

Folkert de Vries (Mar 04 2022 at 19:20):

    fn deref_dec(&self, addr: usize) -> RocDec {
        bits = self.deref_i128(addr)
        ...
    }

Folkert de Vries (Mar 04 2022 at 19:20):

in the definition of

pub trait ReplAppMemory {

Brian Carroll (Mar 04 2022 at 19:21):

wouldn't we need separate methods for i128 and Dec?

Brian Carroll (Mar 04 2022 at 19:21):

oh right sorry, that's what you wrote

Brian Carroll (Mar 04 2022 at 19:21):

I misread

Folkert de Vries (Mar 04 2022 at 19:21):

yeah

Folkert de Vries (Mar 04 2022 at 19:21):

just, we only need to implement deref_dec once in this way

Brian Carroll (Mar 04 2022 at 19:22):

Right, that's a good idea!

Brendan Hansknecht (Mar 04 2022 at 20:40):

Folkert de Vries said:

we can represent 1/3 exactly I think right? and floats cannot

No, but if I remember correctly, our dec is essentially a x * 10^-18. So anything divisible by 10^-18 is always perfectly represented. So 1/3 is still a problem.

Folkert de Vries (Mar 04 2022 at 20:50):

wait then what problem do decimals solve?

Folkert de Vries (Mar 04 2022 at 20:50):

I thought they were designed to prevent this sort of 0.0000000001 thing

Brendan Hansknecht (Mar 04 2022 at 20:59):

Representing cash and any sort of numbers divisible by 10 is the main thing.

Brendan Hansknecht (Mar 04 2022 at 21:01):

Theoretically we could have the base to make it nicer for other cases. Could make it base 60 so it gets 1/2, 1/3, 1/4, 1/5, 1/6, 1/10, 1/12. That gets most human expected fractions and still makes it work well with base 10 since it is a multiple of 10.

Brendan Hansknecht (Mar 04 2022 at 21:03):

Change the number to x * 60^10

Brendan Hansknecht (Mar 04 2022 at 21:07):

Also, I guess it still has a slightly more reasonable third already. Would be 0.33333....3 with 18 3s. So it shouldn't have as many issues with the 0.000000001 thing.

Derek Gustafson (Mar 04 2022 at 21:24):

If I remember correctly, 0.3 isn't exactly representable in base 2.

Folkert de Vries (Mar 04 2022 at 21:24):

ah that must be what I'm thinking of

Derek Gustafson (Mar 04 2022 at 21:27):

If we're looking to provide nice representations for lots of fractions, we probably want it to be a fractional class, although that will be representable in Roc itself once Abilities are implemented.

Brendan Hansknecht (Mar 04 2022 at 21:30):

Ah yeah, it fixes the 0.3 case and like 0.1 + 0.9 actually equalling 1.

Richard Feldman (Mar 04 2022 at 21:31):

yeah in every language that uses IEEE floats if you put 0.1 + 0.2 into the repl, it does not print 0.3 :sweat_smile:

Richard Feldman (Mar 04 2022 at 21:31):

but withDec in Roc it would!

Folkert de Vries (Mar 04 2022 at 21:31):

so it's better for currency, but is it good?

Folkert de Vries (Mar 04 2022 at 21:31):

when you calculate interest, it would still be inaccurate right?

Folkert de Vries (Mar 04 2022 at 21:32):

(and potentially accumulating the errors to a point where they matter)

Richard Feldman (Mar 04 2022 at 21:33):

I remember asking some people about this

Richard Feldman (Mar 04 2022 at 21:35):

I forget exactly who told me this, but it's something like "nobody in finance uses fixed-point decimal with more than 6 decimal places, and one time we had a client ask for 8 or 9 and it was the most absurd thing, everyone was laughing about it"

Brian Carroll (Mar 04 2022 at 21:35):

depends what inaccurate is. If everyone accepts that interest is always compounded at a certain time interval and rounded off then it's ok

Richard Feldman (Mar 04 2022 at 21:35):

and Dec has 18 decimal places, so...seems like we're ok? :sweat_smile:

Richard Feldman (Mar 04 2022 at 21:36):

SQL Server has a fixed-point decimal type where you can configure the number of decimal places, and the common advice there is to always use 4

Richard Feldman (Mar 04 2022 at 21:38):

I remember asking around for various use cases and nobody I talked to had any use cases they knew of where 18 decimal places wouldn't suffice

Richard Feldman (Mar 04 2022 at 21:39):

(unless of course you're doing arbitrary fractions like 1/3, in which case yeah - probably want a different datatype altogether for that!)

Richard Feldman (Mar 04 2022 at 21:41):

basically the goal with Dec is to have a better default than F32 or F64, which actually works the way people expect it to in terms of decimal math

Richard Feldman (Mar 04 2022 at 21:41):

as opposed to all the weird footguns of floating-point math, currency or otherwise

Richard Feldman (Mar 04 2022 at 21:42):

and still having reasoanble, if not ideal, performance

Richard Feldman (Mar 04 2022 at 21:42):

while still of course having F32 and F64 as opt-in higher-performance alternatives if you care more about math operation performance than about precision (e.g. for graphics coordinates)

Derek Gustafson (Mar 04 2022 at 21:55):

@Brian Carroll It's fairly common to quote daily compounding, and actually use continuous compounding, which adds even more inaccuracies.

Derek Gustafson (Mar 04 2022 at 21:56):

I've managed to get myself confused in the RocDec implementation.

#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
pub struct RocDec(pub i128);

That i128 is never named. How can I reference it?

Folkert de Vries (Mar 04 2022 at 21:57):

roc_dec_value.0

Folkert de Vries (Mar 04 2022 at 21:58):

btw if anyone wants to do a bachelors/masters thesis on the Dec type, its implementation (e.g. we'll also need sin and sqrt ect) and its accuracy tradeoffs, that would be very cool

Derek Gustafson (Mar 04 2022 at 21:58):

:+1:

Folkert de Vries (Mar 04 2022 at 21:58):

oh also speed because we do integer arithmetic but it's more operations than the equivalent float operation that happens in hardware

Folkert de Vries (Mar 04 2022 at 21:59):

cc @Samuel Dubovec

Folkert de Vries (Mar 04 2022 at 21:59):

btw @Derek Gustafson was that field already pub? that seems suspicious

Derek Gustafson (Mar 04 2022 at 22:00):

Yup, already pub

Folkert de Vries (Mar 04 2022 at 22:00):

does anything bad happen when you remove it?

Derek Gustafson (Mar 04 2022 at 22:00):

I'll give it a try and let you know

Folkert de Vries (Mar 04 2022 at 22:00):

I guess we want to construct from an i128 sometimes

Folkert de Vries (Mar 04 2022 at 22:01):

but we could add from_le_bytes

Derek Gustafson (Mar 04 2022 at 22:01):

Since I'm not interested in writing about the Dec implementation, should I leave it alone once I'm done with this feature?

Derek Gustafson (Mar 04 2022 at 22:02):

And, speed wise, Dec should be comparable to f64, if there is a fpu available

Folkert de Vries (Mar 04 2022 at 22:06):

not for more specialized operations right? like sqrt or sin?

Folkert de Vries (Mar 04 2022 at 22:07):

at least I thought those might by "in silicon"

Folkert de Vries (Mar 04 2022 at 22:08):

and no if the material interests you go ahead with implementing. Don't want to block on the chance that someone comes along to do a thesis, but if it works out that way then it would be cool

Derek Gustafson (Mar 04 2022 at 22:09):

It's been a decade since I've worked on that sort of stuff. You're probably right that those are hardware instructions on the fpu, and we'd be implementing in software

Brendan Hansknecht (Mar 04 2022 at 22:36):

If I remember, I think one operation is already slower than F64. I believe it was multiplication because it requires more expensive normalization.

Derek Gustafson (Mar 04 2022 at 22:39):

Probably. The work I was doing was fixed point base 2 arithmatic, so we got the renormalization shr, which is really fast.

Richard Feldman (Mar 04 2022 at 22:40):

from way back when we originally talked about this:

Screen-Shot-2022-03-04-at-5.39.23-PM.png Screen-Shot-2022-03-04-at-5.39.38-PM.png

Derek Gustafson (Mar 04 2022 at 23:04):

@Folkert de Vries Removing pub from RocDec.0 causes compilation to fail in mono with 4 errors. I expect they're fixable using traits that are already defined on RocDec, but one problem at a time. I'll make an issue so it doesn't get lost.

Derek Gustafson (Mar 05 2022 at 00:05):

Do we have a safe mode vs release mode implemented?

Folkert de Vries (Mar 05 2022 at 00:21):

there's multiple levels to this

Folkert de Vries (Mar 05 2022 at 00:22):

so, we can build our compiler with --release, which instructs the rust compiler to perform more optimizations

Folkert de Vries (Mar 05 2022 at 00:22):

but, we can also make our compiler turn Roc code into more optimal code

Folkert de Vries (Mar 05 2022 at 00:22):

with the --optimize flag

Folkert de Vries (Mar 05 2022 at 00:22):

optimizations are always safe btw

Richard Feldman (Mar 05 2022 at 00:23):

so --release is for cargo, and makes the roc executable run faster

Richard Feldman (Mar 05 2022 at 00:23):

and --optimize is an argument you pass to the roc executable to make the compiled roc code go faster

Richard Feldman (Mar 05 2022 at 00:23):

yeah, that :point_up: :big_smile:

Folkert de Vries (Mar 05 2022 at 00:24):

e.g. cargo run -- --optimize --debug examples/benchmarks/Deriv.roc

Folkert de Vries (Mar 05 2022 at 00:24):

here --debug make the roc compiler emit the LLVM bytecode to a file

Folkert de Vries (Mar 05 2022 at 00:25):

you will need an extra tool for that though, using that flag should panic with a link to instructions on how to do that

Derek Gustafson (Mar 05 2022 at 00:33):

So, I'm going to be calling str.from_utf8 which returns a Result. The compiler is building the string from the i128, so if we've done our job right, it should always succeed. It's nice to have that check while we're building the compiler, so we know it failed, but in a release mode, we should probably call str.from_utf8unsafe and skip the check that it's a valid codepoint

Folkert de Vries (Mar 05 2022 at 00:33):

oh we just use unsafe in those cases

Folkert de Vries (Mar 05 2022 at 00:34):

it makes sense here; we have to trust ourselves

Folkert de Vries (Mar 05 2022 at 00:34):

and we add tests ect but we choose speed in this sort of scenario

Derek Gustafson (Mar 05 2022 at 00:35):

Okay. I'm used to C where the preprocessor can switch between those choices at compile time

Folkert de Vries (Mar 05 2022 at 00:40):

I'm sure you can, but :shrug:

Brendan Hansknecht (Mar 05 2022 at 00:41):

To be fair, you can do that in rust as well. Just have to use cfg

Brendan Hansknecht (Mar 05 2022 at 00:41):

Generally though, I don't think that would add much value.

Brian Carroll (Mar 05 2022 at 10:36):

Yeah part of the explanation here is in the way a compiler is structured. It's really only the parser that needs to handle invalid UTF8. If the user input doesn't make it past the parser, then none of the other stages need to worry about it.

Derek Gustafson (Mar 05 2022 at 15:23):

What's the format for 1: Dec? I can think of 3 reasonable answers
1) 1
2) 1.
3) 1.0

Brian Carroll (Mar 05 2022 at 15:26):

I suppose whatever we do for 1: F64 this should be the same?

Derek Gustafson (Mar 05 2022 at 15:38):

That would be option 1)

Ayaz Hafiz (Mar 05 2022 at 15:41):

I think 1) is good for now, unlike other languages (at least today) a float literal can be inferred without having to have a decimal point after it.

Derek Gustafson (Mar 05 2022 at 23:31):

Pushed my RocDec fix and I'm getting a number of failing tests regarding recognizing records with a single Num * in them as Num *.
The error is in the function num_to_ast in repl_eval/src/eval.rs, but I don't understand Content well enough to understand what's going on.
Anyone have a suggestion of what could be going on, and where I should look?

Folkert de Vries (Mar 05 2022 at 23:35):

if you just want to peek at what is in a Content, then dbg!(roc_types::subs::SubsFmtContent(content, subs)) works

Folkert de Vries (Mar 05 2022 at 23:44):

hmm I can reproduce the problem locally and it's odd that your changes would cause this issue

Folkert de Vries (Mar 05 2022 at 23:47):

got it, in eval.rs, change this snippet to

    let (newtype_containers, content) = unroll_newtypes(env, content);
    let content = unroll_aliases(env, content);

    macro_rules! helper {
        ($ty:ty) => {
            app.call_function(main_fn_name, |_, num: $ty| {
                num_to_ast(env, number_literal_to_ast(env.arena, num), content)
            })
        };
    }

Ayaz Hafiz (Mar 05 2022 at 23:47):

I think the moving of helper up to line 281 may be causing the problem, as it seems to be capturing content before unrolling

Folkert de Vries (Mar 05 2022 at 23:47):

crucially, content must be defined before the macro

Folkert de Vries (Mar 05 2022 at 23:48):

yes, exactly

Ayaz Hafiz (Mar 05 2022 at 23:48):

What Folkert said :)

Derek Gustafson (Mar 05 2022 at 23:48):

I assumed that my changes exposed an unhandled case of RocDec

Folkert de Vries (Mar 05 2022 at 23:48):

no this is why shadowing is not allowed in roc

Folkert de Vries (Mar 05 2022 at 23:48):

but it's so convenient sometimes ...

Folkert de Vries (Mar 05 2022 at 23:49):

but yeah you have to be careful when moving macros that they capture the same things

Derek Gustafson (Mar 05 2022 at 23:49):

Should I revert that change, or rename something

Folkert de Vries (Mar 05 2022 at 23:49):

no just swap the lines around

Folkert de Vries (Mar 05 2022 at 23:49):

like in the snippet I posted

Derek Gustafson (Mar 05 2022 at 23:50):

Okay. Will do when I get home

Folkert de Vries (Mar 05 2022 at 23:50):

I'll just push that change then, got it already

Folkert de Vries (Mar 05 2022 at 23:51):

repl tests all succeed at least

Last updated: Jul 06 2025 at 12:14 UTC