std.math.sign: get the sign of a int, float or vector #10281

travisstaloch · 2021-12-06T01:12:08Z

returns -1, 0 or 1
supports vector types
branchless

SpexGuy · 2021-12-06T04:17:46Z

As long as it's switching on type info, might as well add an implementation for floats too :P

travisstaloch · 2021-12-06T09:59:06Z

added float support. left a TODO for vectors of floats which is blocked by @inttofloat support for vector types.

travisstaloch · 2021-12-06T23:36:08Z

looks like ci failure was just a timeout

matu3ba

Looks solid, but tests should be improved for the quirky floating numbers and small integers.

matu3ba · 2021-12-20T21:49:51Z

lib/std/math.zig

@@ -1484,3 +1484,66 @@ test "boolMask" {
 pub fn comptimeMod(num: anytype, denom: comptime_int) IntFittingRange(0, denom - 1) {
    return @intCast(IntFittingRange(0, denom - 1), @mod(num, denom));
 }
+
+/// returns -1, 0 or 1.  supports integer types, vectors of integer types, and float types


nit: can you improve this to say what -1, 0 and 1 mean for the documentation?

matu3ba · 2021-12-20T21:52:54Z

lib/std/math.zig

+
+test "sign" {
+    inline for ([_]type{ i8, i32, i64, u8, f16, f32, f64 }) |T| {
+        const input_expecteds = [_][2]comptime_int{


s/input_expecteds/input_expected

matu3ba · 2021-12-20T21:54:37Z

lib/std/math.zig

+            }
+        }
+    }
+    comptime {


Is sign comptime-only? Otherwise you want comptime + runtime tests. You can put them in a function and call one at comptime and the other at runtime.

matu3ba · 2021-12-20T21:56:34Z

lib/std/math.zig

+}
+
+test "sign" {
+    inline for ([_]type{ i8, i32, i64, u8, f16, f32, f64 }) |T| {


You may want to also test these numbers: i0, i1, i2 and u0,u1,u2

matu3ba · 2021-12-20T22:00:21Z

lib/std/math.zig

+    comptime {
+        try std.testing.expectEqual(-1, sign(-10));
+        try std.testing.expectEqual(1, sign(10));
+        try std.testing.expectEqual(0, sign(0));


If you want to add more edge cases:
Add -0.0, 0.0 for floats (yes -0.0 is a valid floating point number), -inf, inf and NaN from here. Or mention the behavior in tests.
If you want to go the extra mile, add 2 denormal numbers (+ and -), but it should be fine without.

daurnimator · 2022-01-01T16:05:48Z

I struggle to see a use case for this function

Returning 0 for zero doesn't really allow for the signed-ness of floats (where there is both a positive and negative 0)
When would I use this function instead of writing e.g. x > 0

iddev5 · 2022-02-20T12:54:18Z

In order to solve the float confusion, the function behavior can be changed as follows:

sign() would return -1, 0 or 1 normally on integer inputs
for floats, it would return -1 for negative floats and -0, and it will return 1 for positive floats and 0. The float version would not return 0 under any case.

Behavior inspired from rust and java strictmath

SpexGuy · 2022-02-20T16:40:43Z

Sign as defined originally (0.0 and -0.0 return 0) is a common primitive in shading languages. (Though metal additionally specifies that sign(-0.0) is -0.0).
HLSL: documentation for sign
GLSL: documentation for sign
Metal: language spec pdf (section 6.2)
The primitive is useful for branchless programing, especially on platforms where multiplication is cheap and float->int conversion or reinterpretation is free (aka GPUs).

andrewrk

I have taken the liberty to squash your 3 commits into 1 and rebase against latest master.

I would be happy to add std.math.sign as you have defined it here, but I would like the following enhancements first:

avoid using inline for in unit tests
use inline on the sign function, since "branchless" is one of the features

travisstaloch · 2022-05-11T07:54:10Z

Thanks @andrewrk. I changed the loops from inline to comptime and made math.sign an inline function. Hope this is what you meant. Let me know if this needs additional changes. 👍

andrewrk

Well now it's only being tested at compile time, and not getting as much runtime coverage. Sorry for not being clear, what I mean is to avoid using loops entirely in your unit tests. This makes it easy to see exactly what cases are being tested, and comment some of them out, stack traces become more useful, etc.

Get rid of input_expecteds. Get rid of the list of types. Just hard code every test case directly, and don't over engineer it. You can use function calls to simplify the logic.

Here's an example of what I am looking for:

zig/lib/std/math.zig

Lines 778 to 809 in 090461a

    
           test "divCeil" { 
        
               try testDivCeil(); 
        
               comptime try testDivCeil(); 
        
           } 
        
           fn testDivCeil() !void { 
        
               try testing.expectEqual(@as(i32, 2), divCeil(i32, 5, 3) catch unreachable); 
        
               try testing.expectEqual(@as(i32, -1), divCeil(i32, -5, 3) catch unreachable); 
        
               try testing.expectEqual(@as(i32, -1), divCeil(i32, 5, -3) catch unreachable); 
        
               try testing.expectEqual(@as(i32, 2), divCeil(i32, -5, -3) catch unreachable); 
        
               try testing.expectEqual(@as(i32, 0), divCeil(i32, 0, 5) catch unreachable); 
        
               try testing.expectEqual(@as(u32, 0), divCeil(u32, 0, 5) catch unreachable); 
        
               try testing.expectError(error.DivisionByZero, divCeil(i8, -5, 0)); 
        
               try testing.expectError(error.Overflow, divCeil(i8, -128, -1)); 
        
               try testing.expectEqual(@as(f32, 0.0), divCeil(f32, 0.0, 5.0) catch unreachable); 
        
               try testing.expectEqual(@as(f32, 2.0), divCeil(f32, 5.0, 3.0) catch unreachable); 
        
               try testing.expectEqual(@as(f32, -1.0), divCeil(f32, -5.0, 3.0) catch unreachable); 
        
               try testing.expectEqual(@as(f32, -1.0), divCeil(f32, 5.0, -3.0) catch unreachable); 
        
               try testing.expectEqual(@as(f32, 2.0), divCeil(f32, -5.0, -3.0) catch unreachable); 
        
               try testing.expectEqual(6, divCeil(comptime_int, 23, 4) catch unreachable); 
        
               try testing.expectEqual(-5, divCeil(comptime_int, -23, 4) catch unreachable); 
        
               try testing.expectEqual(-5, divCeil(comptime_int, 23, -4) catch unreachable); 
        
               try testing.expectEqual(6, divCeil(comptime_int, -23, -4) catch unreachable); 
        
               try testing.expectError(error.DivisionByZero, divCeil(comptime_int, 23, 0)); 
        
               try testing.expectEqual(6.0, divCeil(comptime_float, 23.0, 4.0) catch unreachable); 
        
               try testing.expectEqual(-5.0, divCeil(comptime_float, -23.0, 4.0) catch unreachable); 
        
               try testing.expectEqual(-5.0, divCeil(comptime_float, 23.0, -4.0) catch unreachable); 
        
               try testing.expectEqual(6.0, divCeil(comptime_float, -23.0, -4.0) catch unreachable); 
        
               try testing.expectError(error.DivisionByZero, divCeil(comptime_float, 23.0, 0.0)); 
        
           }

One final request: please make this only a single commit, to make merging easier for me 🙏

andrewrk · 2022-05-11T21:50:11Z

btw isn't this just return copysign(1, x) ?

travisstaloch · 2022-05-11T23:51:55Z

Apologies for my sloppy git skills. I did my best to 'squash' them into one.

btw isn't this just return copysign(1, x) ?

Not sure about using copysign. First i've heard of it. Does it support vectors? If you think that would be better to use for float non-vectors, sounds good to me.

iddev5 · 2022-05-12T01:05:42Z

btw isn't this just return copysign(1, x) ?

The range of signum is {-1, 0, 1} while copysign(1, x) will just return -1 or +1 depending on sign.

Copysign right now is unimplemented for integers and vectors too.

andrewrk · 2022-05-12T01:09:29Z

@iddev5 I think all the math functions should be improved to take anytype and handle vectors and integers where appropriate, and that includes copysign. However, your point about the set of possible output values is spot on.

iddev5 · 2022-05-12T01:11:50Z

One more things is that signum would be defined different for float values than int values. See discussion above. Sadly different languages implement it differently (that behavior of +0 and -0). This is not something which signum suffers from in mathematics.

andrewrk

This is good to merge once the CI is green. Note to person merging: upon merging fix the commit message

travisstaloch force-pushed the std.math.sign branch 3 times, most recently from fece009 to 42f51ab Compare December 6, 2021 01:43

travisstaloch force-pushed the std.math.sign branch from c04b7ec to 523f5f2 Compare December 6, 2021 10:02

travisstaloch changed the title ~~std.math.sign: get the sign of a signed int or vector of signed ints~~ std.math.sign: get the sign of a int or vector Dec 6, 2021

travisstaloch changed the title ~~std.math.sign: get the sign of a int or vector~~ std.math.sign: get the sign of a int, float or vector Dec 6, 2021

matu3ba suggested changes Dec 20, 2021

View reviewed changes

daurnimator added the standard library This issue involves writing Zig code for the standard library. label Jan 1, 2022

Vexu mentioned this pull request Feb 20, 2022

stdlib: signum function #10938

Closed

andrewrk force-pushed the std.math.sign branch from 0c93c5e to d04e7dd Compare May 11, 2022 01:20

andrewrk requested changes May 11, 2022

View reviewed changes

travisstaloch requested a review from andrewrk May 11, 2022 07:49

andrewrk requested changes May 11, 2022

View reviewed changes

andrewrk added the enhancement Solving this issue will likely involve adding new logic or components to the codebase. label May 11, 2022

travisstaloch force-pushed the std.math.sign branch 2 times, most recently from 853d9ca to b81ff35 Compare May 11, 2022 23:44

travisstaloch requested a review from andrewrk May 11, 2022 23:53

andrewrk approved these changes May 12, 2022

View reviewed changes

add std.math.sign

825ede4

andrewrk force-pushed the std.math.sign branch from b81ff35 to 825ede4 Compare May 12, 2022 20:09

andrewrk merged commit 974af5f into ziglang:master May 12, 2022

IntegratedQuantum mentioned this pull request Nov 11, 2022

Implement std.math.sign for float vectors. #13521

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

std.math.sign: get the sign of a int, float or vector #10281

std.math.sign: get the sign of a int, float or vector #10281

travisstaloch commented Dec 6, 2021

SpexGuy commented Dec 6, 2021

travisstaloch commented Dec 6, 2021

travisstaloch commented Dec 6, 2021

matu3ba left a comment

matu3ba Dec 20, 2021

matu3ba Dec 20, 2021

matu3ba Dec 20, 2021

matu3ba Dec 20, 2021

matu3ba Dec 20, 2021 •

edited

Loading

daurnimator commented Jan 1, 2022

iddev5 commented Feb 20, 2022 •

edited

Loading

SpexGuy commented Feb 20, 2022

andrewrk left a comment

travisstaloch commented May 11, 2022

andrewrk left a comment •

edited

Loading

andrewrk commented May 11, 2022 •

edited

Loading

travisstaloch commented May 11, 2022 •

edited

Loading

iddev5 commented May 12, 2022 •

edited

Loading

andrewrk commented May 12, 2022

iddev5 commented May 12, 2022

andrewrk left a comment

	test "divCeil" {
	try testDivCeil();
	comptime try testDivCeil();
	}
	fn testDivCeil() !void {
	try testing.expectEqual(@as(i32, 2), divCeil(i32, 5, 3) catch unreachable);
	try testing.expectEqual(@as(i32, -1), divCeil(i32, -5, 3) catch unreachable);
	try testing.expectEqual(@as(i32, -1), divCeil(i32, 5, -3) catch unreachable);
	try testing.expectEqual(@as(i32, 2), divCeil(i32, -5, -3) catch unreachable);
	try testing.expectEqual(@as(i32, 0), divCeil(i32, 0, 5) catch unreachable);
	try testing.expectEqual(@as(u32, 0), divCeil(u32, 0, 5) catch unreachable);
	try testing.expectError(error.DivisionByZero, divCeil(i8, -5, 0));
	try testing.expectError(error.Overflow, divCeil(i8, -128, -1));

	try testing.expectEqual(@as(f32, 0.0), divCeil(f32, 0.0, 5.0) catch unreachable);
	try testing.expectEqual(@as(f32, 2.0), divCeil(f32, 5.0, 3.0) catch unreachable);
	try testing.expectEqual(@as(f32, -1.0), divCeil(f32, -5.0, 3.0) catch unreachable);
	try testing.expectEqual(@as(f32, -1.0), divCeil(f32, 5.0, -3.0) catch unreachable);
	try testing.expectEqual(@as(f32, 2.0), divCeil(f32, -5.0, -3.0) catch unreachable);

	try testing.expectEqual(6, divCeil(comptime_int, 23, 4) catch unreachable);
	try testing.expectEqual(-5, divCeil(comptime_int, -23, 4) catch unreachable);
	try testing.expectEqual(-5, divCeil(comptime_int, 23, -4) catch unreachable);
	try testing.expectEqual(6, divCeil(comptime_int, -23, -4) catch unreachable);
	try testing.expectError(error.DivisionByZero, divCeil(comptime_int, 23, 0));

	try testing.expectEqual(6.0, divCeil(comptime_float, 23.0, 4.0) catch unreachable);
	try testing.expectEqual(-5.0, divCeil(comptime_float, -23.0, 4.0) catch unreachable);
	try testing.expectEqual(-5.0, divCeil(comptime_float, 23.0, -4.0) catch unreachable);
	try testing.expectEqual(6.0, divCeil(comptime_float, -23.0, -4.0) catch unreachable);
	try testing.expectError(error.DivisionByZero, divCeil(comptime_float, 23.0, 0.0));
	}

std.math.sign: get the sign of a int, float or vector #10281

std.math.sign: get the sign of a int, float or vector #10281

Conversation

travisstaloch commented Dec 6, 2021

SpexGuy commented Dec 6, 2021

travisstaloch commented Dec 6, 2021

travisstaloch commented Dec 6, 2021

matu3ba left a comment

Choose a reason for hiding this comment

matu3ba Dec 20, 2021

Choose a reason for hiding this comment

matu3ba Dec 20, 2021

Choose a reason for hiding this comment

matu3ba Dec 20, 2021

Choose a reason for hiding this comment

matu3ba Dec 20, 2021

Choose a reason for hiding this comment

matu3ba Dec 20, 2021 • edited Loading

Choose a reason for hiding this comment

daurnimator commented Jan 1, 2022

iddev5 commented Feb 20, 2022 • edited Loading

SpexGuy commented Feb 20, 2022

andrewrk left a comment

Choose a reason for hiding this comment

travisstaloch commented May 11, 2022

andrewrk left a comment • edited Loading

Choose a reason for hiding this comment

andrewrk commented May 11, 2022 • edited Loading

travisstaloch commented May 11, 2022 • edited Loading

iddev5 commented May 12, 2022 • edited Loading

andrewrk commented May 12, 2022

iddev5 commented May 12, 2022

andrewrk left a comment

Choose a reason for hiding this comment

matu3ba Dec 20, 2021 •

edited

Loading

iddev5 commented Feb 20, 2022 •

edited

Loading

andrewrk left a comment •

edited

Loading

andrewrk commented May 11, 2022 •

edited

Loading

travisstaloch commented May 11, 2022 •

edited

Loading

iddev5 commented May 12, 2022 •

edited

Loading