search

chapel-lang / chapel

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

Math module - potential renaming for params #19000

Closed lydia-duncan closed 1 year ago

lydia-duncan commented 2 years ago

Here's a proposed renaming. Alternate proposals and discussion welcome

old name new name
e stays the same
log2_e log2E
log10_e log10E
ln_2 ln2 (or logE2 if we decide to use logE instead of ln in https://github.com/chapel-lang/chapel/issues/18995)
ln_10 ln10 (or logE10, ditto connection above)
pi stays the same
half_pi halfPi
quarter_pi quarterPi
recipr_pi reciprPi
twice_recipr_pi twiceReciprPi
twice_recipr_sqrt_pi twiceReciprSqrtPi
sqrt_2 sqrt2
recipr_sqrt_2 reciprSqrt2
ben-albrecht commented 2 years ago

I am in favor of removing the pi variants and sqrt, log, and ln constants (couldn't those be supported via param functions?). I think these were added before these computations could be done within a param.

For reference, here are the math constants that a couple other languages provide:

Python:

pi, e, tau, inf, nan

Julia:

pi, e, eulergamma, catalan, golden
lydia-duncan commented 2 years ago

(I was going to put that in a separate issue)

damianmoz commented 2 years ago

Chapel already has some existing rules used to construct a param identifier and some of them are quite sound, especially when it comes to compound constants.

A constant which is a logarithm, say K, would be computed as

K = log[B](C)

where B is the base which defaults to e if is omitted but could be otherwise be 2 or 10, . The existing Chapel rule that would create a param identifier name for K (which is not unique to Chapel) is that use to create log2_e and log10_e above. It looks at the right hand side of the expression for K and then removes the trailing parenthesis and replaces the leading parenthesis by an underscore. So you end up with a param having a naming convention in this case like

param log[B]_C = ......

I suggest that this existing Chapel rule be kept and even used more widely here, i.e. I would vote to keep the first two the same.

Let's see how that rule goes for the other examples.

One can express a current Chapel naming rule for a reciprocal identifier if one imagines that a function exists called recipr to create a reciprocal, i.e.

recipr(x);

The name of the actual param then created using the same existing Chapel rule as seen with the earlier logarithm and square root constants, i.e. strip the trailing parenthesis and replace the leading one by an underscore.

So given a compile type symbolic constant

param name = .....

the rule to create the identifier of its reciprocal can be expressed as prepending recipr_ to the name is now the first of 

param recipr_name = ...
param rcp_name = ....

where the second assumes a function name of rcp because I wanted to use less characters.

Such an approach, which is consistent with the previous also preserves the name of the original constant, i.e. one can continue to do

grep pi source.chpl

to find all the occurences of pi or _reciprpi.

A related existing Chapel rules for creating a constant name for say a multiple of something like

two * pi

is simply to replace the multiply sign by an underscore and remove the spaces. That same rule preserves that ability to search, because you have names like half_pi and quarter_pi. That also retains symmetry with the original, something you do not have with

param pi = ....
param reciprPi = ....

Sticking to the prepending rules means that for the cases about, you would end up with

param pi = ....
param half_pi = ....
param quarter_pi = ...
param recipr_pi = ....
param recipr_half_pi = ....
param recipr_quarter_pi = ....

This has applied existing Chapel rules for creating consistent param identifier names, albeit more rigorously, and with the benefit of being easily able to search for occurrences of the name and its relations in both source code and symbol tables.

So basically I would vote to keep the existing middle batch much the same, just with more consistency and rigour.

Continuing to apply that same existing Chapel rule rigorously across the board also means that something like a param constant for

sqrt(2)

strip the last parenthesis and change the first to an underscore and we have

param sqrt_2 = 1.41421356237309504880168872420969807856967187537694807317667973799073; // A002193 (OEIS)

which agrees with the current practice.

And ditto for recipr_sqrt_2.

So now I would vote to keep the same existing rule for the last two.

That looks like a vote for a rigorous interpretation of the existing rules for the lot.

So, my vote is to keep the good existing Chapel param identifier naming rules, just apply them consistently. They work - well

Get rid of the bad rules for sure such as that recipr in the midde.

Leave camel casing for the names of a non-compound constant like the Euler-Mascheroni constant

param eulerMascheroniDec = 0.57721566490153286060651209008240243104215933593992;
//or
param eulerMascheroniHex = 0x0.93C467E37DB0C7A4D1BE3F810152CB56A1CECC3Ap0;
damianmoz commented 2 years ago

I just realised that somebody might use the above rules for params to affect routine names, specifically catching in this wide net the routine rsqrt(x) defined, but not calculated as

proc rsqrt(x) return sqrt(x);

That name has been around for ages. But if this renaming is going to affect the style rules (a.k.a. guide) being pursued so that it affects everything, at most I would want to see

proc r_sqrt(x) return 1 / sqrt(x);

and for consistency, that would mean changing the prefix recipr_ or rcp_ in the earlier comments to just r_

lydia-duncan commented 1 year ago

Prep information for an upcoming ad-hoc subteam discussion.

Note that I’m considering the ln and log ones handled by the log discussion in https://github.com/chapel-lang/chapel/issues/18995#issuecomment-1622471631

1. What is the API being presented?

/* e - exp(1) or  the base of the natural logarithm */
param e = 2.7182818284590452354;
/* pi - the circumference/the diameter of a circle */
param pi = 3.14159265358979323846;
/* pi/2 */
param half_pi = 1.57079632679489661923;
/* pi/4 */
param quarter_pi = 0.78539816339744830962;
/* 1/pi */
param recipr_pi = 0.31830988618379067154;
/* 2/pi */
param twice_recipr_pi = 0.63661977236758134308;
/* 2/sqrt(pi) */
param twice_recipr_sqrt_pi = 1.12837916709551257390;
/* sqrt(2) */
param sqrt_2 = 1.41421356237309504880;
/* 1/sqrt(2) */
param recipr_sqrt_2 = 0.70710678118654752440;

How is it intended to be used?

e and pi are the constants with those names. The renaming constants are intended to cover pre-computed values that would be commonly used.

How is it being used in Arkouda and CHAMPS?

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

Were shuffled around a bit due to the split into AutoMath/Math - used to be included by default in all programs, but now aren’t.

They used to live at the end of the module. They were added in June 2015, with some having different names in the first draft of the PR. pi and e were always the same, but:

The PR mentions that these were based on the constants defined in math.h on POSIX systems, and that if param reals were computed at compile time (which I think they are now), we would only need e and pi. 


3. What's the precedent in other languages, if they support it? As relevant, we especially care about:

a. Python

Only pi and e are listed in Python’s math module’s constants list: https://docs.python.org/3/library/math.html#constants. They also provide tau

b. C/C++

IEEE (https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/math.h.html) and Gnu’s (https://www.gnu.org/software/libc/manual/html_node/Mathematical-Constants.html) version define the constants as:

Which matches the ones initially proposed in the PR that added them to Chapel.

c. Rust

Rust provides (https://doc.rust-lang.org/std/f64/consts/index.html):

d. Swift

I think Swift just provides pi (https://developer.apple.com/documentation/swift/floatingpoint/pi), it doesn’t even seem to provide e or tau, let alone the other constants.

e. Julia

Julia just provides pi (https://docs.julialang.org/en/v1/base/numbers/#Base.MathConstants.pi) and e (https://docs.julialang.org/en/v1/base/numbers/#Base.MathConstants.%E2%84%AF) of these, I wasn’t seeing the other constants.

f. Go

Go provides (https://pkg.go.dev/math#pkg-constants):

4. Are there known Github issues with the feature?

Other issues:

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

The ones involving the square root of something are related notionally to the sqrt() function. Naming that doesn’t match that function’s name should probably be avoided, but it won’t otherwise impact it.

pi is used in nbody and quarter_pi is used in our c-ray examples, so those programs will need to be updated if anything changes.


6. How do you propose to solve the problem? 


A.

old name new name
e stays the same
pi stays the same
half_pi halfPi
quarter_pi quarterPi
recipr_pi reciprPi
twice_recipr_pi twiceReciprPi
twice_recipr_sqrt_pi twiceReciprSqrtPi
sqrt_2 sqrt2
recipr_sqrt_2 reciprSqrt2

Might be useful to extend the comments to include names from other languages, so that it is easier for a user looking for what they’re familiar with in that language to find the Chapel equivalent. That said, if such languages undergo a change like we are with our 2.0 effort or choose to deprecate them, we would find ourselves out of date.

B. Deprecate the pure computation versions (not e or pi, and not the log/ln ones)

C. Mark the pure computation versions unstable (not e or pi)

I think we should do both A and C.

damianmoz commented 1 year ago

Again, very well documented.

lydia-duncan commented 1 year ago

In our subteam discussion today, we decided to use the proposed new names and mark these constants unstable:

It was pointed out that the compiler can't actually compute the sqrt ones today because sqrt is not a param function (oops, that was sloppy on my part). But we decided to mark them as unstable anyways.