Skip to content

Commit

Permalink
(Mostly) Unify Math/AutoMath argument names (chapel-lang#22837)
Browse files Browse the repository at this point in the history
[reviewed by @jabraham17]

We decided in a recent discussion that we wanted to fix the argument
names in the Math and AutoMath modules so that they were consistent
with each other and most other modules.  This means that:
- All `i`, `r`, `z`, `im`, and `val` arguments will get turned into `x`,
  including `val` in `logBasePow2`.
- All `m`, `n` and `a`, `b` pairs will get transformed into `x`, `y`
pairs.
- `logBasePow2` will also change its `baseLog2` argument to `exp`

Implements part of the decision in chapel-lang#19005.

Deprecates all the arguments that would need to be changed to comply
with this decision in the AutoMath module (except in functions that are
already deprecated due to moving to the Math module).  Deprecates the
arguments in `logBasePow2` because it was moved more than 1 release
ago.

Note: this PR does not add deprecation messages for the functions that
were recently moved to the Math module so they would no longer be
included by default.  This is because doing so results in resolution
conflicts with the deprecations in AutoMath: we want to maintain
deprecations for two releases and it seems unlikely that any of these
functions are being called with named arguments in the first place,
due to only having a single argument (in the case of things like `cos`)
or the argument ordering not mattering (in the case of `gcd`).  The only
exception is `ldexp`, which may get renamed entirely as a result of
discussion in chapel-lang#19021 and so will be put off until we know the result of
that discussion (because then the version with the old arguments won't
need to be marked as "last resort" to generate the deprecation message
and so won't conflict with the version in AutoMath)

Note: also does not rename the arguments to `isclose`.  This is because
`isclose` itself is also getting renamed, so will do that at the same
time in a separate PR.

Removes the old deprecation for `logBasePow2` since it has been
deprecated for a few releases and leaving it would cause conflicts. In
doing so, left the helper function implementing it in place due to the
deprecated version of `log2` relying on it (and added a note to those
functions to ensure it will get cleaned up when they are removed).

Adds deprecation tests when deprecations were added.  Only two tests
ended up needing modifications as a result of this change - the
submitted mandelbrots now have a slightly different deprecation
message due to extending the message for divceil/etc, which we
stopped including by default in the current release cycle.

Passed a full paratest with futures
  • Loading branch information
lydia-duncan authored Jul 31, 2023
2 parents 40fcbd4 + db4c859 commit 61ec4b3
Show file tree
Hide file tree
Showing 23 changed files with 521 additions and 221 deletions.
352 changes: 259 additions & 93 deletions modules/standard/AutoMath.chpl

Large diffs are not rendered by default.

242 changes: 134 additions & 108 deletions modules/standard/Math.chpl

Large diffs are not rendered by default.

15 changes: 15 additions & 0 deletions test/deprecated/Math/argNames/absIRImZ.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
use Math;

var one = -1;
var two: uint = 2;
var three = -3.0;
var four: imag(64) = -4.0i;
var five: imag(32) = -5.0i;
var six = -6 + 0i;
writeln(abs(i=one));
writeln(abs(i=two));
writeln(abs(i=-3)); // Should trigger the param version
writeln(abs(r=three));
writeln(abs(im=four));
writeln(abs(im=five));
writeln(abs(z=six));
14 changes: 14 additions & 0 deletions test/deprecated/Math/argNames/absIRImZ.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
absIRImZ.chpl:9: warning: The argument name 'i' is deprecated for 'abs', please use 'x' instead
absIRImZ.chpl:10: warning: The argument name 'i' is deprecated for 'abs', please use 'x' instead
absIRImZ.chpl:11: warning: The argument name 'i' is deprecated for param function 'abs', please use 'x' instead
absIRImZ.chpl:12: warning: The argument name 'r' is deprecated for 'abs', please use 'x' instead
absIRImZ.chpl:13: warning: The argument name 'im' is deprecated for 'abs', please use 'x' instead
absIRImZ.chpl:14: warning: The argument name 'im' is deprecated for 'abs', please use 'x' instead
absIRImZ.chpl:15: warning: The argument name 'z' is deprecated for 'abs', please use 'x' instead
1
2
3
3.0
4.0
5.0
6.0
11 changes: 11 additions & 0 deletions test/deprecated/Math/argNames/cargZ.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Note: snippet taken from test/types/complex/diten/cplxMathFnTypes.chpl
proc testTypes(x: complex(?w)) {
const res2 = carg(z=x);
assert(res2.type == real(w/2));
}

var c64 = 1.0:real(32) + 2.0i:imag(32);
var c128 = 1.0: real(64) + 2.0i: imag(64);

testTypes(c64);
testTypes(c128);
4 changes: 4 additions & 0 deletions test/deprecated/Math/argNames/cargZ.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
cargZ.chpl:2: In function 'testTypes':
cargZ.chpl:3: warning: The argument name 'z' is deprecated for 'carg', please use 'x' instead
cargZ.chpl:2: In function 'testTypes':
cargZ.chpl:3: warning: The argument name 'z' is deprecated for 'carg', please use 'x' instead
6 changes: 6 additions & 0 deletions test/deprecated/Math/argNames/conjgZ.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
var c = 3.0 - 2.0i;
writeln(conjg(z=c));
writeln(conjg(z=3.0i));
writeln(conjg(z=2));
writeln(conjg(z=7: uint));
writeln(conjg(z=-4.0));
10 changes: 10 additions & 0 deletions test/deprecated/Math/argNames/conjgZ.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
conjgZ.chpl:2: warning: The argument name 'z' is deprecated for 'conjg', please use 'x' instead
conjgZ.chpl:3: warning: The argument name 'z' is deprecated for 'conjg', please use 'x' instead
conjgZ.chpl:4: warning: The argument name 'z' is deprecated for 'conjg', please use 'x' instead
conjgZ.chpl:5: warning: The argument name 'z' is deprecated for 'conjg', please use 'x' instead
conjgZ.chpl:6: warning: The argument name 'z' is deprecated for 'conjg', please use 'x' instead
3.0 + 2.0i
-3.0i
2
7
-4.0
11 changes: 11 additions & 0 deletions test/deprecated/Math/argNames/cprojZ.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Snippet taken from test/types/complex/diten/cplxMathFnTypes.chpl
proc testTypes(x: complex(?w)) {
const res4 = cproj(z=x);
assert(res4.type == complex(w));
}

var c64 = 1.0:real(32) + 2.0i:imag(32);
var c128 = 1.0: real(64) + 2.0i: imag(64);

testTypes(c64);
testTypes(c128);
4 changes: 4 additions & 0 deletions test/deprecated/Math/argNames/cprojZ.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
cprojZ.chpl:2: In function 'testTypes':
cprojZ.chpl:3: warning: The argument name 'z' is deprecated for 'cproj', please use 'x' instead
cprojZ.chpl:2: In function 'testTypes':
cprojZ.chpl:3: warning: The argument name 'z' is deprecated for 'cproj', please use 'x' instead
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use Math;

for i in 1..4**3 {
writeln("i = ", i, "; log4(i) = ", log4(i));
}

proc log4(x) do return logBasePow2(x, 2);
proc log4(x) do return logBasePow2(val=x, baseLog2=2);
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
depLogBasePow2.chpl:5: In function 'log4':
depLogBasePow2.chpl:5: warning: logBasePow2 is no longer included by default, please 'use' or 'import' the 'Math' module to call it
depLogBasePow2.chpl:2: called as log4(x: int(64))
logBasePow2-ValBaseLog2.chpl:7: In function 'log4':
logBasePow2-ValBaseLog2.chpl:7: warning: The 'val' and 'baseLog2' argument names are now deprecated, please use 'x' and 'exp' respectively
logBasePow2-ValBaseLog2.chpl:4: called as log4(x: int(64))
i = 1; log4(i) = 0
i = 2; log4(i) = 0
i = 3; log4(i) = 0
Expand Down
5 changes: 5 additions & 0 deletions test/deprecated/Math/argNames/modMN.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
writeln(mod(m=6, n=3)); // Should trigger the param version

var x = 6;
var y = 5;
writeln(mod(m=x, n=y));
4 changes: 4 additions & 0 deletions test/deprecated/Math/argNames/modMN.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
modMN.chpl:1: warning: The argument names 'm' and 'n' are deprecated for param 'mod', please use 'x' and 'y' instead
modMN.chpl:5: warning: The argument names 'm' and 'n' are deprecated for 'mod', please use 'x' and 'y' instead
0
1
7 changes: 7 additions & 0 deletions test/deprecated/Math/argNames/sgnI.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
use Math;

var one = -1;
var two: uint = 0;
writeln(sgn(i=one));
writeln(sgn(i=two));
writeln(sgn(i=3)); // Should trigger the param version
6 changes: 6 additions & 0 deletions test/deprecated/Math/argNames/sgnI.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
sgnI.chpl:5: warning: The argument name 'i' is deprecated for 'sgn', please use 'x' instead
sgnI.chpl:6: warning: The argument name 'i' is deprecated for 'sgn', please use 'x' instead
sgnI.chpl:7: warning: The argument name 'i' is deprecated for param 'sgn', please use 'x' instead
-1
0
1
5 changes: 5 additions & 0 deletions test/deprecated/Math/argNames/sqrtZ.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
var c1: complex(64) = 9.0 + 0.0i;
var c2: complex(128) = 16.0 + 0.0i;

writeln(sqrt(z=c1));
writeln(sqrt(z=c2));
4 changes: 4 additions & 0 deletions test/deprecated/Math/argNames/sqrtZ.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
sqrtZ.chpl:4: warning: The argument name 'z' is deprecated for 'sqrt', please use 'x' instead
sqrtZ.chpl:5: warning: The argument name 'z' is deprecated for 'sqrt', please use 'x' instead
3.0 + 0.0i
4.0 + 0.0i
16 changes: 8 additions & 8 deletions test/deprecated/Math/depDivCeilDivFloor.good
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
depDivCeilDivFloor.chpl:1: In function 'launch':
depDivCeilDivFloor.chpl:3: warning: In an upcoming release 'divceil' will no longer be included by default, please 'use' or 'import' the Math module to call it
depDivCeilDivFloor.chpl:5: warning: In an upcoming release 'divfloor' will no longer be included by default, please 'use' or 'import' the Math module to call it
depDivCeilDivFloor.chpl:3: warning: In an upcoming release 'divceil' will no longer be included by default, please 'use' or 'import' the Math module to call it. Additionally its argument names will also change with this move
depDivCeilDivFloor.chpl:5: warning: In an upcoming release 'divfloor' will no longer be included by default, please 'use' or 'import' the Math module to call it. Additionally its argument names will also change with this move
depDivCeilDivFloor.chpl:8: called as launch(a: int(32), b: uint(64))
depDivCeilDivFloor.chpl:1: In function 'launch':
depDivCeilDivFloor.chpl:3: warning: In an upcoming release 'divceil' will no longer be included by default, please 'use' or 'import' the Math module to call it
depDivCeilDivFloor.chpl:5: warning: In an upcoming release 'divfloor' will no longer be included by default, please 'use' or 'import' the Math module to call it
depDivCeilDivFloor.chpl:3: warning: In an upcoming release 'divceil' will no longer be included by default, please 'use' or 'import' the Math module to call it. Additionally its argument names will also change with this move
depDivCeilDivFloor.chpl:5: warning: In an upcoming release 'divfloor' will no longer be included by default, please 'use' or 'import' the Math module to call it. Additionally its argument names will also change with this move
depDivCeilDivFloor.chpl:9: called as launch(a: uint(32), b: uint(64))
depDivCeilDivFloor.chpl:1: In function 'launch':
depDivCeilDivFloor.chpl:3: warning: In an upcoming release 'divceil' will no longer be included by default, please 'use' or 'import' the Math module to call it
depDivCeilDivFloor.chpl:5: warning: In an upcoming release 'divfloor' will no longer be included by default, please 'use' or 'import' the Math module to call it
depDivCeilDivFloor.chpl:3: warning: In an upcoming release 'divceil' will no longer be included by default, please 'use' or 'import' the Math module to call it. Additionally its argument names will also change with this move
depDivCeilDivFloor.chpl:5: warning: In an upcoming release 'divfloor' will no longer be included by default, please 'use' or 'import' the Math module to call it. Additionally its argument names will also change with this move
depDivCeilDivFloor.chpl:10: called as launch(a: uint(64), b: int(32))
depDivCeilDivFloor.chpl:1: In function 'launch':
depDivCeilDivFloor.chpl:3: warning: In an upcoming release 'divceil' will no longer be included by default, please 'use' or 'import' the Math module to call it
depDivCeilDivFloor.chpl:5: warning: In an upcoming release 'divfloor' will no longer be included by default, please 'use' or 'import' the Math module to call it
depDivCeilDivFloor.chpl:3: warning: In an upcoming release 'divceil' will no longer be included by default, please 'use' or 'import' the Math module to call it. Additionally its argument names will also change with this move
depDivCeilDivFloor.chpl:5: warning: In an upcoming release 'divfloor' will no longer be included by default, please 'use' or 'import' the Math module to call it. Additionally its argument names will also change with this move
depDivCeilDivFloor.chpl:11: called as launch(a: uint(64), b: uint(32))
divceil(7:int(32),5:uint(64)) = 2
divfloor(7:int(32),5:uint(64)) = 1
Expand Down
8 changes: 4 additions & 4 deletions test/deprecated/Math/depDivCeilPos.good
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
depDivCeilPos.chpl:1: warning: In an upcoming release 'divceilpos' will no longer be included by default, please 'use' or 'import' the Math module to call it
depDivCeilPos.chpl:2: warning: In an upcoming release 'divceilpos' will no longer be included by default, please 'use' or 'import' the Math module to call it
depDivCeilPos.chpl:3: warning: In an upcoming release 'divceilpos' will no longer be included by default, please 'use' or 'import' the Math module to call it
depDivCeilPos.chpl:4: warning: In an upcoming release 'divceilpos' will no longer be included by default, please 'use' or 'import' the Math module to call it
depDivCeilPos.chpl:1: warning: In an upcoming release 'divceilpos' will no longer be included by default, please 'use' or 'import' the Math module to call it. Additionally its argument names will also change with this move
depDivCeilPos.chpl:2: warning: In an upcoming release 'divceilpos' will no longer be included by default, please 'use' or 'import' the Math module to call it. Additionally its argument names will also change with this move
depDivCeilPos.chpl:3: warning: In an upcoming release 'divceilpos' will no longer be included by default, please 'use' or 'import' the Math module to call it. Additionally its argument names will also change with this move
depDivCeilPos.chpl:4: warning: In an upcoming release 'divceilpos' will no longer be included by default, please 'use' or 'import' the Math module to call it. Additionally its argument names will also change with this move
2
4
2
Expand Down
8 changes: 4 additions & 4 deletions test/deprecated/Math/depDivFloorPos.good
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
depDivFloorPos.chpl:1: warning: In an upcoming release 'divfloorpos' will no longer be included by default, please 'use' or 'import' the Math module to call it
depDivFloorPos.chpl:2: warning: In an upcoming release 'divfloorpos' will no longer be included by default, please 'use' or 'import' the Math module to call it
depDivFloorPos.chpl:3: warning: In an upcoming release 'divfloorpos' will no longer be included by default, please 'use' or 'import' the Math module to call it
depDivFloorPos.chpl:4: warning: In an upcoming release 'divfloorpos' will no longer be included by default, please 'use' or 'import' the Math module to call it
depDivFloorPos.chpl:1: warning: In an upcoming release 'divfloorpos' will no longer be included by default, please 'use' or 'import' the Math module to call it. Additionally its argument names will also change with this move
depDivFloorPos.chpl:2: warning: In an upcoming release 'divfloorpos' will no longer be included by default, please 'use' or 'import' the Math module to call it. Additionally its argument names will also change with this move
depDivFloorPos.chpl:3: warning: In an upcoming release 'divfloorpos' will no longer be included by default, please 'use' or 'import' the Math module to call it. Additionally its argument names will also change with this move
depDivFloorPos.chpl:4: warning: In an upcoming release 'divfloorpos' will no longer be included by default, please 'use' or 'import' the Math module to call it. Additionally its argument names will also change with this move
2
4
1
Expand Down
Binary file modified test/studies/shootout/submitted/mandelbrot.pbm
Binary file not shown.
Binary file modified test/studies/shootout/submitted/mandelbrot3.pbm
Binary file not shown.

0 comments on commit 61ec4b3

Please sign in to comment.