gleam-lang / stdlib

🎁 Gleam's standard library
https://hexdocs.pm/gleam_stdlib/
Apache License 2.0
492 stars 175 forks source link

Add `to_precision` to `gleam/float` #668

Closed versecafe closed 2 months ago

versecafe commented 3 months ago

Example

float.to_precision(2.43434348473, precision: 2)
// -> 2.43
float.to_precision(547890.453444, precision: -3)
// -> 548000.0
versecafe commented 3 months ago

Doesn't add new JS or Erl but still creates an FFI and is only ~20% slower than the prior solution

versecafe commented 3 months ago

This must be in Gleam.

@lpil It is? the second commit gets rid of the FFI stuff and uses just existing functions, this was what @giacomocavalieri recommended

the new FFI for to_float on int is to prevent a recursive dep on gleam/int which depends on gleam/float

giacomocavalieri commented 3 months ago

I think it's worth including more context here :)

So it all started with Louis rejecting, with good reason, an implementation done entirely in ffi land that would add new erlang/js code to the stdlib.

This sparked a bit of back and forth in the server, the first pure gleam implementation proposed was not as efficient as it could be and I proposed this pure Gleam version:

// 1
pub fn to_precision(float: Float, precision: Int) -> Float {
  let assert Ok(factor) = power(10.0, int.to_float(-precision))
  int.to_float(round(float /. factor)) *. factor
}

Now since this has to be defined inside the gleam/float it can't use int.to_float directly (or it would result in a circular dependency) so it will need to be rewritten as:

// 2
pub fn to_precision(float: Float, precision: Int) -> Float {
  let assert Ok(factor) = power(10.0, to_float(-precision))
  to_float(float.round(float /. factor)) *. factor
}

// With the to_float function calling the already existing ffi function.

I think this is as close as we can get to pure Gleam given we can't import int.to_float.


Now, if performance is a concern, this isn't as fast as we could get: calling power does a few additional checks to protect from errors that can never actually happen here since the base is the constant value 10.0 (hence the assert-ing) so we might as well skip those and call the do_power function directly:

// 3
pub fn to_precision(float: Float, precision: Int) -> Float {
  let factor = do_power(10.0, to_float(-precision))
  to_float(float.round(float /. factor)) *. factor
}

I personally think this is fine since do_power is already defined and used in the module so we might as well use it here. This makes the code ~14x faster and about as fast as the first pure-ffi version that was proposed by Verse.


To sum it up:

I'd be totally fine with either implementation. We could stick with // 2 until someone actually finds that this function is the source of a performance bottleneck or might as well go with // 3 right away.

Anyway, this is the context of how I came up with the function that I suggested Verse to use :)

versecafe commented 2 months ago

@lpil tests now cover negative inputs, negative precisions, and a mix of them

versecafe commented 2 months ago

@lpil changelog is updated and ready to go