Math module - rename divceil family

[lydia-duncan]

div and ceil are two different abbreviations, so this name and divfloor should be camelCased to match our other library functions, e.g. divCeil and divFloor. divCeil does look a little odd to me, but maybe I just need to get used to it.

divceilpos and divfloorpos are a little less clear to me. These variants assume that the result will be positive and of a signed integer type. The brainless renaming would be divCeilPos and divFloorPos, but it seems likely we can think of better names. I'm not coming up with any myself, anyone else?

[damianmoz]

As all these names are essentially operators, I suggest staying away from camel casing. I have never seen the case where name than implies an operator, even if it is implemented with a routine, was written as camel cased.

Some of us stay away from camel casing anything less than 8 (or even 10) letters, and especially when they are made up of 2 or 3 words/abbreviations where readability says that you cannot afford to have any one of those 2 or 3 appear to have more or less emphasis than the other.

My 2c even if it does not follow the Style Rules. I actually thought that Chapel had a Style Guide but they seem to be quoted as Rules.

[lydia-duncan]

Looking at other languages:
C uses divceil

Python uses floor_divide in NumPy and I don't see a ceiling equivalent.

Java uses floorDiv and I don't see a ceiling equivalent

I don't see either combination for Rust, but I do see an unstable_div_floor

I think Julia's fld1 is equivalent to divfloor and I don't see a ceiling equivalent.

So it seems like there's not a consistent ordering between the two parts but that they are frequently viewed as separable either with underscores or with casing. So we're free to make our own choice here, but I do think the operator point is worth considering - I'm going to bring it up on the style guide issue.

I actually thought that Chapel had a Style Guide but they seem to be quoted as Rules.

Sorry, that's probably a communication issue on my part - it should still be considered a guide for users but in aiming for consistency with our standard modules, we're treating it as rules for things that live there. We don't want to dictate that users must write their code a certain way, but if something is going to get contributed to the standard modules in the future, we'd like to have a clear precedent to point to.

[damianmoz]

I though it was C++ which has divceil and divfloor.

Remember that there are 5 rounding modes for which IEEE 754 wants support. But languages which do more than just truncated division, most often only do floored division because that is the most common. I think that Meta has a set of templates that does all five in C++.

Note that ceil and floor are verbs in English, so you ceil and floor an argument but the routines of those names then return a ceiling and floor. So ceil is technically not an abbreviation. English is so weird!

Is the complete list in Chapel just divceil, divfloor, divceilpos, divfloorpos? Are you going to reserve divroundaway and divroundeven? I think divroundtrunc is covered by the operator /. Or do you just use divabove instead of divceil, divbelow instead of divfloor, and then divaway and diveven without the mention of round?

Is these appear in an expression, using capitals in a camel case name distracts the eyes from the expression, and even more so if these are operators. Then again, I find that the amalgamation of more than two words with lowercase has me looking at camelCase.

[damianmoz]

I thought that Python has a real floor'ed division - the operator //. Not that we can use that in Chapel.

[lydia-duncan]

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

1. What is the API being presented?

  /* Returns :proc:`~AutoMath.ceil`\(`m`/`n`),
     i.e., the fraction `m`/`n` rounded up to the nearest integer.

     If the arguments are of unsigned type, then
     fewer conditionals will be evaluated at run time.
  */
  proc divceil(param m: integral, param n: integral) param { … }
  proc divceil(m: integral, n: integral) { … }

  /* Returns :proc:`~AutoMath.floor`\(`m`/`n`),
     i.e., the fraction `m`/`n` rounded down to the nearest integer.

     If the arguments are of unsigned type, then
     fewer conditionals will be evaluated at run time.
  */
  proc divfloor(param m: integral, param n: integral) param { … }
  proc divfloor(m: integral, n: integral) { … }

// And the unstable:
  /*
    A variant of :proc:`divceil` that performs no runtime checks.
    The user must ensure that both arguments are strictly positive
    (not 0) and are of a signed integer type (not `uint`).
  */
proc divceilpos(m: integral, n: integral) { … }
  /*
    A variant of :proc:`divfloor` that performs no runtime checks.
    The user must ensure that both arguments are strictly positive
    (not 0) and are of a signed integer type (not `uint`).
  */
proc divfloorpos(m: integral, n: integral) { … }

How is it intended to be used?

These are convenience functions for combining division and a rounding operation on that result.

The unstable “pos” variants are for extra performance when certain conditions are assured (though they are unstable due to questions about their actual utility in practice).

How is it being used in Arkouda and CHAMPS?

None of these functions are used in Arkouda or CHAMPS

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


divceil and divfloor were added in 2011, and divceil was used in some histogram tests that were previously relying doing explicit ceil calls on divisions.

divceilpos and divfloorpos were originally added in a side module, UtilMath, also in 2011. This module was deleted in 2015 and the two functions merged into the Math module at that time.

Documentation of the functions was added in 2015.

Used to be included in all user programs by default, but we recently decided to stop doing that.

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

a. Python

Searching for operators led me to //, operator.floordiv (https://docs.python.org/3/library/operator.html#operator.floordiv), //=, and operator.ifloordiv (https://docs.python.org/3/library/operator.html#operator.ifloordiv). But I don’t see a ceiling equivalent.

NumPy has floor_divide (https://numpy.org/doc/stable/reference/generated/numpy.floor_divide.html#numpy.floor_divide). I also don’t see a ceiling equivalent, but it does call out divmod (https://numpy.org/doc/stable/reference/generated/numpy.divmod.html#numpy.divmod) as related. NumPy also says that the // operator can be used as shorthand for floor_divide on ndarrays.

There seems to be no equivalent for the pos variants

b. C/C++

Though I asserted above that C had divceil, I’m not finding it when I look today. That’s what I get for not saving the links, I suppose.

c. Rust

Rust provides div_floor (https://doc.rust-lang.org/std/primitive.i64.html#method.div_floor) and div_ceil (https://doc.rust-lang.org/std/primitive.i64.html#method.div_ceil) but both are listed as an experimental API. They are defined on the integer and unsigned integer types of various sizes.

There seems to be no equivalent for the pos variants.

d. Swift

I don’t see equivalents for Swift.

e. Julia

Julia provides fld (https://docs.julialang.org/en/v1/base/math/#Base.fld) and cld (https://docs.julialang.org/en/v1/base/math/#Base.cld). I do not see an equivalent for the pos variants.

Julia also provides fld1 (https://docs.julialang.org/en/v1/base/math/#Base.fld1). I’d have to do more experimenting with the exact behavior, but it seems like for positive numbers, this functions similarly to a ceiling call. Probably worth coming back to when we discuss more extensive rounding support, but less relevant for this decision.

f. Go

I don’t see equivalents in Go.


4. Are there known Github issues with the feature?


• The argument names for these functions will be renamed as a result of the decision in Math module - unifying argument names #19005

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

• Related to division operators and the rounding functions, especially ceil and floor. If we do something like lengthen ceil to ceiling in this context, we'd be inconsistent with the name of the ceil function, but otherwise naming choices probably wouldn't impact this too much.

• BigInteger has divExact, div and divRem (which are in the process of being renamed to those names from divexact, divQ and divQR), but doesn't have these exact functions

  • It does have other combo functions: addMul and subMul, which we (will have?) renamed from addmul and submul


• These functions motivated the request for a way to query paramness: feature request: paramness-query #14639. But naming changes won’t impact that request

• We could consider adding other combinations of rounding modes + division when more extensive rounding support is implemented. We would likely try to match the naming scheme when we do so.

6. How do you propose to solve the problem? 


A. Leave as is

• Pros:
  • Less implementation work (but should be a quick change to make)
  • Less impact to users (but should be a quick change for them)
  • Same ordering as Rust
  • Names are listed in the order in which they are performed
• Cons:
  • These are two distinct abbreviations so they should be camelCased
  • Not consistent with our decision for BigInteger module functions (e.g. divExact, addMul, see [Library Stabilization] Change BigInteger module method names #17724 (comment))
  • Not the name used by Julia or NumPy/Python

B. rename to divCeil, divCeilPos, divFloor, divFloorPos

• Pros:
  • Consistent with our standard module style guide, and with BigInteger combo functions
  • Same ordering as Rust
  • Names are listed in the order in which they are performed
• Cons:
  • Maybe looks a little funky?
  • Not the name used by Julia or NumPy/Python

C. rename divceil and divfloor but leave the pos variants unchanged because they’re unstable and we might get rid of them anyways.

• Pros:
  • Ditto other options, but less work on something that’s already unstable
  • We might come up with a better name for them than divCeilPos when we try to stabilize them, so changing to it now would be wasted effort
• Cons:
  • Leaving them as is, even marked unstable, might confuse users looking for patterns
  • If we do keep them, odds are we’d rename them to match the new names for divceil and divfloor anyways

D. rename to ceilDiv, ceilDivPos, floorDiv and floorDivPos

• Pros:
  • Matches Python’s ordering and Python’s operator naming
  • Names are listed in the order they would appear in code were the functions gone (ceil(x / y))
• Cons:
  • Doesn’t match Rust’s ordering
  • Not the name used by Julia

E. rename to ceilDivide, ceilDividePos, floorDivide, floorDividePos

• Pros:
  • Matches Python’s ordering and NumPy’s naming
  • Names are listed in the order they would appear in code were the functions gone
• Cons:
  • Doesn’t match Rust’s ordering
  • Not the name used by Julia

F. rename to cld, cldPos, fld, and fldPos

• Pros:
  • Short
  • Matches the names used by Julia
• Cons:
  • Pretty obfuscated - have to look it up to know what it means and search the documentation for a word that isn’t in the function name in order to find them
  • Doesn’t match Rust or Python

I would probably follow B. I’m pretty against F and would rather not leave them as is.

[bradcray]

Am I understanding that we'll make divceilpos() and divfloorpos() unstable whatever we call them?

[lydia-duncan]

They have already been marked unstable, so no additional work is needed to make them unstable

[lydia-duncan]

In our discussion today, we decided to camel case all the functions.

In addition to the options listed above, Jade also proposed marking these functions unstable and when we have more extensive rounding support, adding a div function that takes an argument to determine the rounding. E.g.

proc div(x, y, rounding = ceil) // where ceil is an enum constant

We talked a bit about if we would want this to be shared with BigInteger's handling of rounding modes. If it used the same name as the BigInteger enum today, we could move that enum to Math and re-export it from BigInteger to avoid making it a breaking change. But doing so would force us to support all rounding modes for BigInteger as well, which would likely mean implementing some ourselves (which wouldn't be as optimized as the ones supported by GMP).

We ultimately decided against this course of action, so will just camel case the functions.