rewrite math.complex #20346
rewrite math.complex #20346
Conversation
|
Happy to merge the fixes separately as these look good, but will need a bit more time to check over the api changes. I generally think they look promising, but given the proposal isn't accepted and it changes quite a bit want a closer look. |
| const u = @as(f32, @floatFromInt(@as(i32, @intFromFloat(t)))); | ||
| return ((x - u * DP1) - u * DP2) - t * DP3; | ||
| const u = @trunc(t); | ||
| return ((x - u * DP1) - u * DP2) - u * DP3; |
There was a problem hiding this comment.
Replaced unnecessary foat->int->float cast with a single @trunc.
Correction: fixed mistakenly using t when it should be u.
| if (a == 0.0) { | ||
| // overflow | ||
| return Complex(f32).init(maxnum, maxnum); | ||
| } |
There was a problem hiding this comment.
Overflow branch no longer present in upstream.
| if (a == 0.0) { | ||
| // overflow | ||
| return Complex(f32).init(maxnum, maxnum); | ||
| } |
There was a problem hiding this comment.
Overflow branch no longer present in upstream.
| const u = @as(f64, @floatFromInt(@as(i64, @intFromFloat(t)))); | ||
| return ((x - u * DP1) - u * DP2) - t * DP3; | ||
| const u: f64 = @trunc(t); | ||
| return ((x - u * DP1) - u * DP2) - u * DP3; |
There was a problem hiding this comment.
Replaced unnecessary foat->int->float cast with a single @trunc.
Correction: fixed mistakenly using t when it should be u.
There was a problem hiding this comment.
This file is replaced in its entirety with nothing carried over.
API-wise, the following are changed:
- various helper additions, and tests are all rewritten.
- removed:
Complextype constructor. - changed: unary complex functions that output complex types now inherits the the argument type.
- changed: binary complex functions that output complex types now receives an explicit type argument for its output type.
Impl-wise, the following are changed:
- Some functions which used to only support c32 and c64 now have a generic fallback implementations.
- Inverse complex-trig functions that depended on each other as trivial permutations are now instead rewritten from first principles using the primitive operations (log, etc). This also eliminates some problematic choices in upstream where, instead of permuting argument or output components, they add a pi/2 phase shift to values which is imprecise.
| if (math.signbit(x)) { | ||
| return Complex(f32).init(@abs(x - y), math.copysign(x, y)); | ||
| } else { | ||
| return Complex(f32).init(x, math.copysign(y - y, y)); | ||
| } | ||
| return if (math.signbit(x)) .{ | ||
| @abs(y - y), | ||
| math.copysign(x, y), | ||
| } else .{ | ||
| x, | ||
| math.copysign(y - y, y), | ||
| }; |
There was a problem hiding this comment.
Correction: the real component of the have-sign-bit case should be @abs(y - y) instead of @abs(x - y).
| if (math.signbit(x)) { | ||
| return Complex(f64).init(@abs(x - y), math.copysign(x, y)); | ||
| } else { | ||
| return Complex(f64).init(x, math.copysign(y - y, y)); | ||
| } | ||
| return if (math.signbit(x)) .{ | ||
| @abs(y - y), | ||
| math.copysign(x, y), | ||
| } else .{ | ||
| x, | ||
| math.copysign(y - y, y), | ||
| }; |
There was a problem hiding this comment.
Correction: the real component of the have-sign-bit case should be @abs(y - y) instead of @abs(x - y).
| if (iy == 0 and ix >= 0x7f800000) { | ||
| if (hx & 0x7fffff == 0) { | ||
| return Complex(f32).init(x * x, math.copysign(@as(f32, 0.0), x) * y); | ||
| } | ||
| return Complex(f32).init(x, math.copysign(@as(f32, 0.0), (x + x) * y)); | ||
| } | ||
| if (iy == 0 and ix >= 0x7f800000) return .{ | ||
| x * x, | ||
| if (hx & 0x7fffff == 0) | ||
| math.copysign(zero, x) * y | ||
| else | ||
| math.copysign(zero, (x + x) * y), | ||
| }; |
There was a problem hiding this comment.
Correction: the real component of both cases should be x * x instead of the nonzero-x case being different as x.
| return Complex(f32).init(math.copysign(h, x) * @cos(y), h * @sin(y)); | ||
| break :ret .{ | ||
| @cos(y) * h, | ||
| @sin(y) * math.copysign(h, x), | ||
| }; |
There was a problem hiding this comment.
Correction: the multipliers on cos/sin of the real/imag parts are swapped.
| if (ix < 0x7f800000 and iy < 0x7f800000) { | ||
| if (iy == 0) { | ||
| return Complex(f32).init(math.cosh(x), y); | ||
| } | ||
| if (ix < 0x7f800000 and iy < 0x7f800000) return ret: { | ||
| if (iy == 0) break :ret .{ | ||
| math.cosh(x), | ||
| x * y, | ||
| }; | ||
|
|
There was a problem hiding this comment.
Correction: the imag component should be x * y instead of just y.
|
My primary question is what does removing the A user having to create a structurally matching type seems odd. I can't think of any great use-cases that this approach gives over the dedicated type in 99% of cases. We also lose documentation on a base type we could refer to. |
|
Author has not complied with my request to limit to 1 open PR at a time, is now in timeout for 1 year. |
Closes #19207
This decouples the
math.complexnamespace away from a centralized type definition, instead providing a bring-your-own-type collection of utility functions with no dependency on the specific types' declarations.The
math.Complextype constructor is removed, instead encouraging programmers to pick-and-choose the subset of provided functions to incorporate into their type of choice.This remains the case even once #16278 is implemented, as the collection already makes no assumptions about the concrete type beyond the presence of a
reandimfield.The source files are reorganized such that public-facing dispatch interfaces are all inside
complex.zig, the subdirectory instead only containing datatype-specific internal implementations that is only@imported inside the dispatch function body.The datatype-specific internal implementations are entirely oblivious to any sort of type constructs, receiving inputs as multiple scalar arguments and returning tuples of values.
Future Incremental Improvements: