non-Result Math operations · contributing

Hi everyone!
@Kevin Gillette and I are going to work on making math operations no longer return Result. Specifically, these functions are: div, divFloor, divCeil, mod, rem, log, and sqrt.
Here's the corresponding GitHub issue: https://github.com/rtfeldman/roc/issues/2826

Kevin Gillette (Apr 10 2022 at 15:53):

@Nikita Mounier so we can divide work, which functions are you considering to be div-related?

Nikita Mounier (Apr 10 2022 at 15:55):

Sorry you're right, I meant div, divFloor, and divCeil. It's true that mod and rem are also division-related.

Nikita Mounier (Apr 10 2022 at 15:55):

Kevin Gillette (Apr 10 2022 at 15:56):

Yes, I completely meant that as a joke. That I thought about ahead of time. Indeed.

Kevin Gillette (Apr 10 2022 at 15:58):

Okay, sounds good. So you'll handle div, divFloor, and divCeil, and I'll handle mod, rem, log, and sqrt?

Nikita Mounier (Apr 10 2022 at 16:05):

Nikita Mounier (Apr 10 2022 at 16:06):

Should the panic be at the zig level (using the `@panic("...") builtin function I saw elsewhere?

Nikita Mounier (Apr 10 2022 at 16:09):

This is how the div function inside builtins/bitcode/src/dec.zig handles denominators equal to 0:

if (denominator_i128 == 0) {
    // The compiler frontend does the `denominator == 0` check for us,
    // therefore this case is unreachable from roc user code
    unreachable;
}

So should I remove the compiler frontend check and instead have zig panic? Or should the panic be higher up?

Folkert de Vries (Apr 10 2022 at 16:11):

Folkert de Vries (Apr 10 2022 at 16:12):

Folkert de Vries (Apr 10 2022 at 16:13):

Nikita Mounier (Apr 10 2022 at 16:13):

Right, I saw @panic being used for overflow which feels similar to dividing by 0. Cool!

Kevin Gillette (Apr 10 2022 at 16:14):

Nikita Mounier (Apr 10 2022 at 16:14):

Could you point me to the directory where the compiler frontend would do the denominator == 0 check? I'm still not super familiar with the codebase structure

Ayaz Hafiz (Apr 10 2022 at 16:16):

Nikita Mounier (Apr 10 2022 at 16:17):

Kevin Gillette (Apr 10 2022 at 16:31):

What are people's thoughts about the wording of the panics. It looks like all @panic calls within latest trunk start with the word "TODO". It also doesn't look like there are any explicit panics for integer overflow to reference (are those handled by rust/zig intrinsics)?

Kevin Gillette (Apr 10 2022 at 16:34):

Nikita Mounier (Apr 10 2022 at 17:00):

Btw it might be a good idea if we were to both put all the checked variants inside compiler/module/src/symbol.rs, where all the symbols are listed in a macro with a number followed by its name followed by the corresponding function.
For instance, there's:

39 NUM_DIV_FLOAT: "div"
40 NUM_DIV_INT: "divFloor"

39 NUM_DIV_FLOAT: "div"
40 NUM_DIV_FLOAT_CHECKED: "divChecked"
41 NUM_DIV_INT: "divFloor"
42 NUM_DIV_INT_CHECKED: "divFloorChecked"

Since we gotta offset the number on the left each time we add a new symbol, we might get some nasty merge conflicts if we don't take each others' checked functions into account. Do you see where I'm coming from?

Folkert de Vries (Apr 10 2022 at 17:09):

Folkert de Vries (Apr 10 2022 at 17:10):

Kevin Gillette (Apr 10 2022 at 17:14):

Side note: are the integers critical for humans to manage (is there any bespoke "42ness" to "42"), or does other human-managed code refer to the ID by number instead of symbol? At some point could those integer identifiers by assigned by position, thus minimizing conflicts?

Nikita Mounier (Apr 10 2022 at 17:20):

Kevin Gillette (Apr 10 2022 at 17:40):

Thanks! I've got some stuff to take care of over the next several hours, so it sounds like you may well have code I can reference (or at least will touch relevant files before I do).

Something that's not entirely clear to me at this time: why do some operations have _INT or _FLOAT suffices? NUM_DIV_INT makes sense to me, given that it is, iirc, a separate operator, but NUM_MOD_INT vs NUM_MOD_FLOAT is not clear to me given that NUM_REM has no such variants.

Nikita Mounier (Apr 10 2022 at 18:25):

Sure! I can already tell you that some of the files you'll want to modify are compiler/builtins/roc/Num.roc, compiler/builtins/bitcode/src/num.zig, compiler/builtins/bitcode/src/dec.zig, compiler/can/src/builtins.rs, compiler/module/src/low_level.rs, compiler/module/src/symbol.rs, and compiler/builtins/src/std.rs

Nikita Mounier (Apr 10 2022 at 18:57):

There's one thing I'm slightly confused by, if anyone could give me some context. From compiler/module/src/symbol.rs, divFloor is associated to the symbol NUM_DIV_INT. However, in compiler/can/src/builtins.src, NUM_DIV_INT is associated to the num_div_int function, which, in the branch where the denominator is not equal to 0, is identical to num_div_float's success branch. What's weirder, the comment above num_div_int defines it as:

/// Num.div : Int a , Int a -> Result (Int a) [ DivByZero ]*

/// Num.divFloor: Int a , Int a -> Result (Int a) [ DivByZero ]*

Nikita Mounier (Apr 10 2022 at 19:02):

IIRC, modulus for integer and modulus for floats works quite differently. For instance, C++ has % for integer modulus and fmod for floating-point modulus. Most newer languages unified the two though, even though the implementation works differently under the hood whether it's integer or float.

Brendan Hansknecht (Apr 10 2022 at 19:16):

I haven't looked at it, but their is a chance that float div is just a generic llvm division and not actually specific to floating point, thus int can call it

Brendan Hansknecht (Apr 10 2022 at 19:16):

Brendan Hansknecht (Apr 10 2022 at 19:20):

Also int division is like normal int division in other languages where it is not a floor. It also goes towards zero. Floor goes towards -infinity

Nikita Mounier (Apr 10 2022 at 19:24):

Kevin Gillette (Apr 10 2022 at 19:41):

@Nikita Mounier my guess is that this is the difference between the / operator (floating point division) and the // operator (flooring division).

Brendan Hansknecht (Apr 10 2022 at 20:12):

Bug? Differing opinion around rounding direction? :shrug:
Probably just wasn't thought about.

Brendan Hansknecht (Apr 10 2022 at 20:13):

A lot of the symbols were added just to give standard features, they may not be fully correct/thought through. For example, we recently had a large discussion about bit shifting

Nikita Mounier (Apr 10 2022 at 23:31):

Nikita Mounier (Apr 11 2022 at 18:59):

I'm not sure I fully understand the test error CI is giving me about f64 not implementing PartialEq<{integer}. What do you think?

Ayaz Hafiz (Apr 11 2022 at 19:01):

Ayaz Hafiz (Apr 11 2022 at 19:02):

Ayaz Hafiz (Apr 11 2022 at 19:03):

Try making it -1.0 rather than -1 in the test. I think otherwise Rust infers it to be an integer

Nikita Mounier (Apr 11 2022 at 19:44):

Kevin Gillette (Apr 12 2022 at 03:04):

Direction-wise, is this the design intent, or something we intend to change? As I'm working on a PR to handle make mod panic, I might as well either get rid of float mod, or alternatively look into implementing it for floats (or at least add clarifying comments in the code) depending on the answer to this question.

Kevin Gillette (Apr 12 2022 at 04:28):

A bit more clarity needed: % and %% each appear to inconsistently be referred to as mod and rem/modFloor.

| `a % b`           | `Num.rem a b`    |
| `a %% b`          | `Num.mod a b`    |

`a % b` is shorthand for `Num.mod a b`.

`a %% b` is shorthand for `Int.modFloor a b`.

iiuc, the primary difference between integer rem and mod is in the handling of negative operands. Is that Roc's interpretation as well? What then is the intended behavior of modFloor as defined on integers (should it be called mod or rem instead)? And which operator is truly meant to correspond to which named function?

Brendan Hansknecht (Apr 12 2022 at 15:14):

Mostly my assumption that could be wrong. I personally don't think it should work with floats. They can produce quite surprising results when you use mod due to their inaccuracies. I think that in many languages, % doesn't work with float types, but they still have an explicity fmod function somewhere in the standard library.

Nikita Mounier (Apr 12 2022 at 16:25):

fn gen_div_checked_by_zero_dec() {
    assert_evals_to!(
        indoc!(
            r#"
                    x : Dec
                    x = 10

                    y : Dec
                    y = 0

                    when Num.divChecked x y is
                        Ok val -> val
                        Err _ -> -1
                "#
        ),
        -1,
        i128
    );
}

CI says it's returning -1000000000000000000 instead of -1. Any idea why that could be happening?

Brendan Hansknecht (Apr 12 2022 at 16:27):

Brendan Hansknecht (Apr 12 2022 at 16:28):

In rust you are viewing the raw dec value, so it it 10^18 times bigger than you expect.

Nikita Mounier (Apr 12 2022 at 16:33):

I see – should I replace -1 with something like RocDec::from_str_to_i128_unsafe("-1") then?

Brendan Hansknecht (Apr 12 2022 at 16:33):

Yeah, that or the explicit value. I would just follow whatever other dec tests do.

Nikita Mounier (Apr 12 2022 at 16:36):

Yeah, the other dec tests do RocDec::from_str_to_i128_unsafe so I'll just stick with that. Thanks!

Richard Feldman (Apr 12 2022 at 17:16):

I spent a bunch of time looking into remainder and mod at some point; let me look some things up to refresh my memory on the subject :big_smile:

Richard Feldman (Apr 12 2022 at 17:19):

so one of my goals for Roc in general is to try to have the "performance ceiling" be as high as possible without breaking referential transparency or memory safety. That is, try to make there be some way to get the fastest combination of machine instructions into the binary, so that if you find that your Roc program isn't running fast enough, you have some way to solve that problem - even if it involves writing code in a way you normally wouldn't

Richard Feldman (Apr 12 2022 at 17:20):

that goal guides builtin design; for example, if the CPU supports a particular arithmetic operation (like float mod), by default I want to expose that as a primitive so that if someone really needs that for their particular use case to run fast, they have access to it

Richard Feldman (Apr 12 2022 at 17:20):

because if the operation isn't an effect and isn't exposed in the builtins, there's really no other way to access it

Richard Feldman (Apr 12 2022 at 17:22):

so if mod didn't accept Num, then I'd want to have a separate modFloat that only works on floats...which maybe is fine, if mod on floats is indeed a footgun, but it would have the downside that you could no longer do mod on floats using an infix operator, so I'd want to learn more about that before splitting one function into two! :big_smile:

Richard Feldman (Apr 12 2022 at 17:37):

ok, looking it up, it appears that there actually is no CPU instruction for modulo or remainder for floats, so let's drop that operation for now :thumbs_up: - we can revisit adding it again in the future if there's demand, and discuss the pros/cons then

Richard Feldman (Apr 12 2022 at 17:38):

Nikita Mounier (Apr 12 2022 at 18:51):

Nikita Mounier (Apr 12 2022 at 18:53):

I'll make the necessary changes to the documentation, then I think it's ready to merge...

Nikita Mounier (Apr 12 2022 at 19:01):

Kevin Gillette (Apr 12 2022 at 21:02):

@Richard Feldman so that means Roc will no longer have an %% operator? % will desugar to Num.rem, and Num.mod is only available by name?

Richard Feldman (Apr 12 2022 at 21:04):

Jared Cone (Apr 12 2022 at 22:06):

Just wanted to mention some inconsistency that / does float division and // does integer division, but % does int remainder with this proposal?

Brendan Hansknecht (Apr 12 2022 at 22:28):

Just wondering, does / ever cast int to floats, or is it always float division with float inputs. If it never casts, can't we just have / for both float and int division. It will just fail to type check if you use and int with a float?

Kevin Gillette (Apr 12 2022 at 23:28):

Python 3 broke compatibility with Python 2 by changing / to always mean "float division" (previously it mean "flooring division" for ints and "regular division" for floats), while // was introduced to always mean "flooring division". I feel this clarity makes sense.

In Python 3 the types are preserved though for //, such that 3.0 // 2.0 yields 1.0, yet / always produces a float even from int inputs.

I'm not sure about what typing choice is reasonable for Roc. For Elm's tradeoffs (less concerned with access to low-level performance control), I'd think (//) : Num, Num -> Int makes sense, but for Roc, it's not clear how to turn a Num a into an Int b. Also, if someone doing performance sensitive floating point code wanted a truncated float result, forcing them to explicitly convert back to float would be undesirable (even if LLVM could optimize that into remaining a float the whole time).

I would expect divFloor : Num a, Num a -> Num a is reasonable (no type changes).

I don't know what to think about the result type of div however due to the type correspondence question. Do we just prohibit div and / on integers?

Kevin Gillette (Apr 12 2022 at 23:29):

(and does that imply that a function operating on Num can't access /, since not every Num variant would have access to non-flooring division?)

Richard Feldman (Apr 13 2022 at 00:59):

the main motivation for that split is that it's easy to get surprising answers with int division

Richard Feldman (Apr 13 2022 at 01:00):

e.g. in x / y, if x is 3 and y is 2, then if they're both floats the answer is 1.5 but if they're both ints the answer is 1

Richard Feldman (Apr 13 2022 at 01:01):

and 3 / 2 returning 1 is a surprising result because that's not what it is in math

Richard Feldman (Apr 13 2022 at 01:02):

Kevin Gillette (Apr 13 2022 at 04:39):

I128 : Num (Integer Signed128)
I64 : Num (Integer Signed64)
I32 : Num (Integer Signed32)
I16 : Num (Integer Signed16)
I8 : Int Signed8

Of these two styles, which is preferred? It looks like the I8 style was introduced more recently. Would it be desirable to align these declarations on one style or another?

Ayaz Hafiz (Apr 13 2022 at 04:55):

They resolve to the same underlying type (Int is Int a : Num (Integer a)), so there is no functional difference, also because the end user will only ever see I8 or I16 anyway, not the whole expanded type.

In any case these definitions will change relatively soon with opaque types and abilities.

Stream: contributing

Topic: non-Result Math operations

Nikita Mounier (Apr 10 2022 at 15:52):

Kevin Gillette (Apr 10 2022 at 15:53):

Nikita Mounier (Apr 10 2022 at 15:55):

Nikita Mounier (Apr 10 2022 at 15:55):

Kevin Gillette (Apr 10 2022 at 15:56):

Kevin Gillette (Apr 10 2022 at 15:58):

Nikita Mounier (Apr 10 2022 at 16:05):

Nikita Mounier (Apr 10 2022 at 16:06):

Nikita Mounier (Apr 10 2022 at 16:09):

Folkert de Vries (Apr 10 2022 at 16:11):

Folkert de Vries (Apr 10 2022 at 16:12):

Folkert de Vries (Apr 10 2022 at 16:12):

Folkert de Vries (Apr 10 2022 at 16:13):

Nikita Mounier (Apr 10 2022 at 16:13):

Kevin Gillette (Apr 10 2022 at 16:14):

Nikita Mounier (Apr 10 2022 at 16:14):

Ayaz Hafiz (Apr 10 2022 at 16:16):

Nikita Mounier (Apr 10 2022 at 16:17):

Nikita Mounier (Apr 10 2022 at 16:17):

Kevin Gillette (Apr 10 2022 at 16:31):

Kevin Gillette (Apr 10 2022 at 16:34):

Nikita Mounier (Apr 10 2022 at 17:00):

Folkert de Vries (Apr 10 2022 at 17:09):

Folkert de Vries (Apr 10 2022 at 17:10):

Kevin Gillette (Apr 10 2022 at 17:14):

Nikita Mounier (Apr 10 2022 at 17:20):

Nikita Mounier (Apr 10 2022 at 17:20):

Kevin Gillette (Apr 10 2022 at 17:40):

Nikita Mounier (Apr 10 2022 at 18:25):

Nikita Mounier (Apr 10 2022 at 18:57):

Nikita Mounier (Apr 10 2022 at 19:02):

Brendan Hansknecht (Apr 10 2022 at 19:16):

Brendan Hansknecht (Apr 10 2022 at 19:16):

Brendan Hansknecht (Apr 10 2022 at 19:20):

Nikita Mounier (Apr 10 2022 at 19:24):

Kevin Gillette (Apr 10 2022 at 19:41):

Brendan Hansknecht (Apr 10 2022 at 20:12):

Brendan Hansknecht (Apr 10 2022 at 20:13):

Nikita Mounier (Apr 10 2022 at 23:31):

Nikita Mounier (Apr 11 2022 at 18:59):

Ayaz Hafiz (Apr 11 2022 at 19:01):

Ayaz Hafiz (Apr 11 2022 at 19:02):

Ayaz Hafiz (Apr 11 2022 at 19:03):

Nikita Mounier (Apr 11 2022 at 19:44):

Kevin Gillette (Apr 12 2022 at 03:04):

Kevin Gillette (Apr 12 2022 at 04:28):

Brendan Hansknecht (Apr 12 2022 at 15:14):

Nikita Mounier (Apr 12 2022 at 16:25):

Brendan Hansknecht (Apr 12 2022 at 16:27):

Brendan Hansknecht (Apr 12 2022 at 16:28):

Nikita Mounier (Apr 12 2022 at 16:33):

Brendan Hansknecht (Apr 12 2022 at 16:33):

Nikita Mounier (Apr 12 2022 at 16:36):

Richard Feldman (Apr 12 2022 at 17:16):

Richard Feldman (Apr 12 2022 at 17:19):

Richard Feldman (Apr 12 2022 at 17:20):

Richard Feldman (Apr 12 2022 at 17:20):

Richard Feldman (Apr 12 2022 at 17:22):

Richard Feldman (Apr 12 2022 at 17:37):

Richard Feldman (Apr 12 2022 at 17:38):

Richard Feldman (Apr 12 2022 at 17:38):

Nikita Mounier (Apr 12 2022 at 18:51):

Nikita Mounier (Apr 12 2022 at 18:53):

Nikita Mounier (Apr 12 2022 at 18:53):

Nikita Mounier (Apr 12 2022 at 19:01):

Kevin Gillette (Apr 12 2022 at 21:02):

Richard Feldman (Apr 12 2022 at 21:04):

Jared Cone (Apr 12 2022 at 22:06):

Brendan Hansknecht (Apr 12 2022 at 22:28):

Kevin Gillette (Apr 12 2022 at 23:28):

Kevin Gillette (Apr 12 2022 at 23:29):

Richard Feldman (Apr 13 2022 at 00:59):

Richard Feldman (Apr 13 2022 at 01:00):

Richard Feldman (Apr 13 2022 at 01:01):

Richard Feldman (Apr 13 2022 at 01:02):

Richard Feldman (Apr 13 2022 at 01:02):

Kevin Gillette (Apr 13 2022 at 04:39):

Ayaz Hafiz (Apr 13 2022 at 04:55):