chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.79k stars 421 forks source link

Math module - should we rename cbrt? #19007

Closed lydia-duncan closed 1 year ago

lydia-duncan commented 2 years ago

It did take me a minute to realize it stood for cube root. However, that is clear from reading the documentation and it matches sqrt - ideally, the current name would be most clear if it was sorted near sqrt, but it sounds like we were potentially leaning towards including sqrt by default but not cbrt over in #18990 which would make sorting them together less possible.

The name I would use instead would be cubeRoot, which I think is still short enough to be reasonable. Should we rename it, and if so should we use this name? Is there a better option?

damianmoz commented 2 years ago

Again, part of what you say is an old argument.

The name cbrt gained traction when the seminal book, Software Manual for the Elementary Functions appeared. While this excellent text is my go-to book for such a topic, I am sure that the two authors were not so worried about rigor in the names of their functions.

So you are right, the naming ofcbrt does not match sqrt. Digging back into history, a name appeared in Algol68 circles about the same time as that book but saw no traction outside of Algol68 afficiados.

Basically, it was noticedthat the naming of SQuare RooT took the first 2 letters of square and the first and last letters of root. By applying the same naming rules to CUbe RooT means that the name for such functionality is then

proc curt(x : real(w)) .....

Then we have a nice tight name which rigorously follows the naming convention of square root with no problems about camel casing. And we have four decades of precedence or provenance.

And we can have a zero overhead alias, i.e.

inline proc cbrt(x) return curt(x);

for those who want the old name in the list of names which must be explicitly included or used.

bradcray commented 2 years ago

So you are right, the naming of cbrt does not match sqrt.

I don't particularly agree with this—I think they're pretty symmetrical in that they both choose two letters from the term in question, and I think a common way to abbreviate is to focus on consonants over vowels or important sounds over initial sounds.

but it sounds like we were potentially leaning towards including sqrt by default but not cbrt over in #18990 which would make sorting them together less possible.

I think we should sort them together, whichever side of the fence they land on. I'm not sure I grokked what cbrt was on #18990 (though I'm seeing now that you did define it), so didn't comment on it there.

lydia-duncan commented 1 year ago

Prep for an upcoming ad-hoc subteam (scheduled for the two week sprint starting August 8th)

1. What is the API being presented?

  /* Returns the cube root of the argument `x`. */
extern proc cbrt(x: real(64)): real(64);
inline proc cbrt(x : real(32)): real(32) { … }

The name is not necessarily immediately apparent, but it is clear from leading the documentation and sorting it near sqrt would help make it more obvious (as well as maybe contextual clues near where it was used).

How is it intended to be used?

Used to find the cube root for the argument (pretty self-explanatory)

How is it being used in Arkouda and CHAMPS?

Not used in Arkouda.

Used twice in CHAMPS:

2. What's the history of the feature, if it already exists?

The 64 bit version was added in 2006, alongside a bunch of other math functions. It predated the use of real as the type name.

The 32 bit version was added in 2007, along with a bunch of other 32 bit functions.

These functions predated the inline and extern keywords, as well as using proc for functions.

Documentation for these functions was added in 2015.

Also in 2015, we attempted to use a unified interface instead of the separate real(64)/real(32) versions but was thwarted by issues with promotion.

Was shuffled around in the recent Math/AutoMath split - now lives in AutoMath so it can continue to be included in all programs by default.


3. What's the precedent in other languages, if they support it?

a. Python

Python provides cbrt (https://docs.python.org/3/library/math.html#math.cbrt).

b. C/C++

C and C++ provide three versions of cbrt, with different names for the different sizes of floating point numbers used (cbrt, cbrtf, cbrtl, https://en.cppreference.com/w/cpp/numeric/math/cbrt).

I believe C/C++ is where we get the naming of our version from.

c. Rust

Rust uses cbrt (https://doc.rust-lang.org/std/primitive.f64.html#method.cbrt)

d. Swift

Swift doesn’t seem to provide this function. It does have squareRoot instead of sqrt (https://developer.apple.com/documentation/swift/double/squareroot())

e. Julia

Julia uses cbrt (https://docs.julialang.org/en/v1/base/math/#Base.Math.cbrt)

f. Go

Go uses Cbrt (https://pkg.go.dev/math#Cbrt)


4. Are there known Github issues with the feature?


5. Are there features it is related to? What impact would changing or adding this feature have on those other features?


6. How do you propose to solve the problem?


A. Leave it as is

B. Rename to cubeRoot

I think we should leave it as is.

damianmoz commented 1 year ago

The abbreviation rule for sqrt is sq[uare] r[oo]t. It has been around for 60+ years (it is part of Algol 60)

The current cbrt uses a kill-the-vowels abbreviation. It has been around since the 70s.

If you follow the 2019 IEEE 754 recommendation to have a generic root, i.e. n'th root amd follow the same abbreviation rule as for sqrt(), you could consider ge[neric] r[oo]t or gert(). This also yields fort(), firt(), sirt(), *sert(), eirt() and nirt() for the 4th through 9th roots. Total consistency with the original sqrt(). All four letters. All read well in an equation.

It you apply that consistent abbreviation rule to a cube root, you get cu[be] r[oo]t, or curt(). This rolls off the tongue, not least because it is an English word (with no meaning here of course). In a mathematical expression, curt() reads a lot better than cuberoot() (and even cbrt()).

I cannot claim any credit for the abbreviation. The name curt() has been in Algol68 for 39 years and maybe a bit longer.

Note that IEEE 754 mandates only the sqrt() functionality, along side the 4 arithmetic operators, +, -, * and /. Nothing else. Every other root, from the 3rd upwards is just recommended. So if you like to follow IEEE 754, the cube root probably should live in Math, not AutoMath.

My 2c.

P.S. In discussions about the name squareRoot(), I got everything from rolled eyes, to serious complaints about how it lengthens too many expressions and everything in between. A long name with a capital in the middle forces your eyes on that name when you really want to be able to have the whole expression in your eyeball's field of view in terms of validating correctness (or potentially making a bug-free change). Swift has so much going for it but there are decisions where it shoots itself in the foot.

bradcray commented 1 year ago

I think we should leave it as is.

I tend to agree.

lydia-duncan commented 1 year ago

In our discussion today, we were strongly in favor of keeping the name as is