[Proposal] Add Num.cbrt

[PureFox48]

The lack of a built-in cube root function has been causing me a problem lately.

I had been using x.pow(1/3) until I realized that this doesn't work when x is negative because Num.pow then returns NaN. So I'm now using (in my Math library):

static cbrt(x) { (x >= 0) ? x.pow(1/3) : -(-x).pow(1/3) }

which works fine even for infinities or NaN.

However, it would be nice if this function were 'on tap' given that it could be implemented by simply calling the C99 library function, cbrt(x), which should execute much more quickly than a Wren implementation. It may also use a better algorithm than my 'naive' version above.

Although cube roots don't come up as often as square roots, they're still quite common in mathematical work which is probably why they're in the C99 (and also Go) standard libraries in the first place.

A worthwhile addition? Comments welcome.

[mhermier]

Can be trivially added in core or as a foreign helper. The only thing that annoy me is the lack of reusablility that lots of `Num` methods signature has. Because of implementation details it can become an issue. One side note, maybe you can speed up your implementation a bit by doing something like: `num.sign * num.abs.pow(1/3)`

[PureFox48]

Thanks for that suggestion.

I just tried it and it's definitely faster for a small number of calls but gets slower as the number of calls increases, possibly because internally there's always three method calls instead of one.

Incidentally, does Wren ever inline anything such as simple property getters?

I was wondering how expensive it is to use field where possible rather than _field to cut down on the number of leading underscores which tends to make expressions harder to read.

[mhermier]

If you need the patch, it can be done in less than 5 minutes. No, inlining is an advanced feature that the wren trivial compiler will never be able to do. It is not hard to read it just a matter of getting used to.

[PureFox48]

Yeah, that's what I suspected given that the compiler is only single pass.

At least though method calls are fast relative to comparable languages.

Whilst personally I'd like to see Num.cbrt, I don't want to clutter up the Num class unless others would find it useful too.

[mhermier]

From d857342fb807ccc968a9c0f7b0f66f3433337b46 Mon Sep 17 00:00:00 2001
From: Michel Hermier <michel.hermier@gmail.com>
Date: Mon, 21 Dec 2020 19:27:37 +0100
Subject: [PATCH 1/1] wren/core: Add `Num::sqrt`.

---
 src/vm/wren_core.c         | 2 ++
 test/core/number/cbrt.wren | 6 ++++++
 2 files changed, 8 insertions(+)
 create mode 100644 test/core/number/cbrt.wren

diff --git a/src/vm/wren_core.c b/src/vm/wren_core.c
index 1ef92fab..1abfff68 100644
--- a/src/vm/wren_core.c
+++ b/src/vm/wren_core.c
@@ -677,6 +677,7 @@ DEF_NUM_FN(abs,     fabs)
 DEF_NUM_FN(acos,    acos)
 DEF_NUM_FN(asin,    asin)
 DEF_NUM_FN(atan,    atan)
+DEF_NUM_FN(cbrt,    cbrt)
 DEF_NUM_FN(ceil,    ceil)
 DEF_NUM_FN(cos,     cos)
 DEF_NUM_FN(floor,   floor)
@@ -1345,6 +1346,7 @@ void wrenInitializeCore(WrenVM* vm)
   PRIMITIVE(vm->numClass, "acos", num_acos);
   PRIMITIVE(vm->numClass, "asin", num_asin);
   PRIMITIVE(vm->numClass, "atan", num_atan);
+  PRIMITIVE(vm->numClass, "cbrt", num_cbrt);
   PRIMITIVE(vm->numClass, "ceil", num_ceil);
   PRIMITIVE(vm->numClass, "cos", num_cos);
   PRIMITIVE(vm->numClass, "floor", num_floor);
diff --git a/test/core/number/cbrt.wren b/test/core/number/cbrt.wren
new file mode 100644
index 00000000..5f76b4a2
--- /dev/null
+++ b/test/core/number/cbrt.wren
@@ -0,0 +1,6 @@
+System.print(8.cbrt)        // expect: 2
+System.print(1000000.cbrt)  // expect: 100
+System.print(1.cbrt)        // expect: 1
+System.print((-0).cbrt)     // expect: -0
+System.print(0.cbrt)        // expect: 0
+System.print(-2.cbrt)       // expect: -1.2599210498949
-- 
2.29.2

[PureFox48]

Hey, thanks for doing that :)

Looks good to go. Think I can manage the docs myself:

cbrt

The cube root of the number.

[ruby0x1]

fwiw I don't think cbrt is a good naming if this is added. cube would be a more meaningful choice as a quick example, this abbreviated mess of letters isn't elegant, and doesn't fit.

[PureFox48]

Well, cbrt is analogous to sqrt which we already have.

I don't think you could use cube(x) as that suggests x * x * x.

[PureFox48]

I just had a quick look around to see what other languages use and every one I looked at which has one - C++, Java, C#, Rust, Javascript, Ruby and Python(numpy) - use the name cbrt for the cube root function.

So, elegant or not, at least it would be familiar to most people.

I suppose you could use cubeRoot but it wouldn't then sit well with sqrt.

[MasFlam]

Honestly I've never in my life seen the cube root function named differently than cbrt. As far as it being an ugly name... Well maybe it isn't the prettiest, but any other name would come as even more weird or confusing

[ruby0x1]

Yea it's reasonable as convention similar to sqrt, apparently I've never used this function so wasn't familiar with it's presence in other languages, my comment stemmed from that.

[mhermier]

Well all these names comes from a time completion was not really a thing, space was costly, and utf-8 was a dream. These days, we could also have used the utf-8 character to have an square/cube root unary operators, same thing with greek symbols.

[ChayimFriedman2]

Well all these names comes from a time completion was not really a thing, space was costly, and utf-8 was a dream. These days, we could also have used the utf-8 character to have an square/cube root unary operators, same thing with greek symbols.

Yeah however:

1. Programming languages tend to avoid Unicode symbols for operations, because they're hard to type.
2. As stated above, programmers get used to those short names.

[mhermier]

Sure, what I meant was that all in all `cbrt`, `cubic_root`, `3√`, proper unicode symbol, ... It does not matter, as long as it follows some logic/convention/... to help memories it. Human is the most adaptable machine ever, but if we can help it a little it is better.

[PureFox48]

Thought I'd just add that Gravity also has a Math.cbrt function.

They also have Math.xrt though I think that would be going a bit too far for Wren's core library.

[ChayimFriedman2]

Math.xrt(x, y) in Gravity is just y.pow(1 / x) in Wren.

[PureFox48]

Precisely :)

[PureFox48]

Actually, that wouldn't work if y were negative as you'd then get NaN even for odd roots. You'd have to use a similar workaround to what we used for cbrt in the opening posts instead.

However, it's still not worth adding IMO as roots above the third don't crop up very often in my experience.

[mhermier]

Just noticed there is a typo in my comment for the patch, will issue a PR.