[stdlib] FloatLiteral missing operators to conform Python float types

[msaelices]

There are some missing operators in the FloatLiteral struct that we would need to implement to conform the Python float type:

• __round__(ndigits)
• __mod__(rhs)
• __divmod__(rhs)

This PR implements them and adds some tests to check the methods.

[laszlokindrat]

This is awesome, thank you for pushing on this! I have a couple of questions and mostly small quibbles and suggestions, but we highly appreciate this improvement!

[laszlokindrat]

Can you please add the Roundable trait explicitly to FloatLiteral? That will probably require you to split this into two signatures. After that, would you be interested in changing the __round__ signature in Roundable to expect ndigits as well?

[msaelices]

Implemented Roundable trait here: msaelices@26906ca

I'm not sure if adding a ndigits function to Roundable would break integers, and that parameter on those types has no meaning, right?

[laszlokindrat]

It has the same meaning as for floating point numbers, it's just that for non-negative ndigits values, the round function acts like identity on integers:

round(5232323,-2) == 5232300

If you are interested, I would love to see a followup patch with improving the Roundable trait and the implementation for other types!

[msaelices]

Sure. I will create a separated PR with the improvement.

[laszlokindrat]

Amazing, thanks, please ping me when that's done!

[msaelices]

@laszlokindrat It's here: #2656

[laszlokindrat]

Maybe there is a compiler bug, but this can't work for a different reason: FloatLiteral is nonmaterializable, so you cannot have a runtime variable with that type. This would be one possible way to work around this:

alias PairType = Tuple[FloatLiteral, FloatLiteral]
alias t: PairType = FloatLiteral(4.5).__divmod__(2.0)
assert_equal(t[0], 2.0)  # not sure if direct indexing works on `Tuple` yet, but you get the point
assert_equal(t[1], 0.5)

[msaelices]

It does not work:

    alias Pair = Tuple[FloatLiteral, FloatLiteral]
    alias t: Pair = (4.5).__divmod__(2.0)
    assert_equal(t[0], 2.0)
    assert_equal(t[1], 0.5)

Gives me the following error:

RUN: at line 13: mojo /home/msaelices/src/mojo/stdlib/test/builtin/test_float_literal.mojo
+ mojo /home/msaelices/src/mojo/stdlib/test/builtin/test_float_literal.mojo
/home/msaelices/src/mojo/stdlib/test/builtin/test_float_literal.mojo:124:18: error: invalid call to '__init__': argument #1 cannot be converted from 'Pair' to 'Reference['Tuple[FloatLiteral, FloatLiteral]', ...]
    assert_equal(t[0], 2.0)
                 ^
/home/msaelices/src/mojo/stdlib/test/builtin/test_float_literal.mojo:124:18: note: cannot bind an RValue to a reference
    assert_equal(t[0], 2.0)
                 ^
/home/msaelices/src/mojo/stdlib/src/memory/reference.mojo:222:8: note: function declared here
    fn __init__(inout self, value: Self._mlir_type):
       ^
/home/msaelices/src/mojo/stdlib/test/builtin/test_float_literal.mojo:125:18: error: invalid call to '__init__': argument #1 cannot be converted from 'Pair' to 'Reference['Tuple[FloatLiteral, FloatLiteral]', ...]
    assert_equal(t[1], 0.5)
                 ^
/home/msaelices/src/mojo/stdlib/test/builtin/test_float_literal.mojo:125:18: note: cannot bind an RValue to a reference
    assert_equal(t[1], 0.5)
                 ^
/home/msaelices/src/mojo/stdlib/src/memory/reference.mojo:222:8: note: function declared here
    fn __init__(inout self, value: Self._mlir_type):

And, trying to avoid the usage of indexing on Tuples I did this:

    alias Pair = Tuple[FloatLiteral, FloatLiteral]
    alias t: Pair = (4.5).__divmod__(2.0)
    assert_equal(t, (2.0, 0.5))

Getting this error:

RUN: at line 13: mojo /home/msaelices/src/mojo/stdlib/test/builtin/test_float_literal.mojo
+ mojo /home/msaelices/src/mojo/stdlib/test/builtin/test_float_literal.mojo
/home/msaelices/src/mojo/stdlib/test/builtin/test_float_literal.mojo:124:17: error: no matching function in call to 'assert_equal'
    assert_equal(t, (2.0, 0.5))
    ~~~~~~~~~~~~^~~~~~~~~~~~~~~
/home/msaelices/src/mojo/stdlib/src/testing/testing.mojo:118:4: note: candidate not viable: callee expects 1 parameter, but 0 were specified
fn assert_equal[T: Testable](lhs: T, rhs: T, msg: String = "") raises:
   ^
/home/msaelices/src/mojo/stdlib/test/builtin/test_float_literal.mojo:124:18: note: failed to infer parameter 'T', argument type 'Tuple[FloatLiteral, FloatLiteral]' does not conform to trait 'Testable'
    assert_equal(t, (2.0, 0.5))
                 ^
/home/msaelices/src/mojo/stdlib/src/testing/testing.mojo:139:4: note: candidate not viable: argument #0 cannot be converted from 'Pair' to 'String'
fn assert_equal(lhs: String, rhs: String, msg: String = "") raises:
   ^
/home/msaelices/src/mojo/stdlib/src/testing/testing.mojo:156:4: note: candidate not viable: callee expects 2 parameters, but 0 were specified
fn assert_equal[
   ^
/home/msaelices/src/mojo/stdlib/test/builtin/test_float_literal.mojo:124:17: note: failed to infer parameter 'type', parameter isn't used in any argument
    assert_equal(t, (2.0, 0.5))
    ~~~~~~~~~~~~^~~~~~~~~~~~~~~
mojo: error: failed to parse the provided Mojo source module

[laszlokindrat]

I'm gonna take a look at why this is happening.

[msaelices]

I've figured it out. There was a minor typo in your suggestions. When I saw it again I've found that the second alias was actually a var declaration, so it's fixed here: msaelices@5914b4c

[msaelices]

Thanks for the alias Pair trick, it worked like a charm

[laszlokindrat]

great, thank you!

[laszlokindrat]

I'm actually suprised this worked for you: var t: Pair = (4.5).__divmod__(2.0) shouldn't compile since FloatLiteral is non-materializable. On a related note, I'm running into issues with this in our internal CI. I'll continue to investigate.

[laszlokindrat]

Yeah, something is wrong with Tuple in parameter expressions. I will land your patch with slightly modified tests, and we can circle back later if needed.

[laszlokindrat]

This is awesome, thank you for pushing on this! I have a couple questions and quibbles, but we really appreciate this contribution!

[laszlokindrat]

Thanks again, it's great to have this fixed!

[modularbot]

✅🟣 This contribution has been merged 🟣✅

Your pull request has been merged to the internal upstream Mojo sources. It will be reflected here in the Mojo repository on the nightly branch during the next Mojo nightly release, typically within the next 24-48 hours.

We use Copybara to merge external contributions, click here to learn more.

[modularbot]

Landed in 5f22b5f! Thank you for your contribution 🎉