-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add initial complex-number support #949
Conversation
- Library type instead of builtin - All C complex functions implemented Partial WIP: Needs more tests for edge cases.
std/math/complex/sqrt.zig
Outdated
const cmath = math.complex; | ||
const Complex = cmath.Complex; | ||
|
||
// NOTE: Returning @typeOf(z) here causes issues when trying to access the value. This is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was there a current issue regarding this, cannot recall?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks related to #733.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you referring to the fact that if you pass a struct to a parameter that accepts var
then it ends up being a &const S
instead of S
?
I don't think that will affect calling with c.re
vs calling with re
. I'll run the tests locally to make sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is an example. Release mode actually crashes with a SIGSEGV here. May not be 100% the same issue.
const std = @import("std");
fn Complex(comptime T: type) type {
return struct {
const Self = this;
re: T,
im: T,
pub fn new(re: T, im: T) Self {
return Self {
.re = re,
.im = im,
};
}
};
}
fn clone1(z: var) @typeOf(z) {
const T = @typeOf(z.re);
return Complex(T).new(z.im, z.im);
}
fn clone2(z: var) Complex(@typeOf(z.re)) {
const T = @typeOf(z.re);
return Complex(T).new(z.im, z.im);
}
pub fn main() void {
const s = Complex(f32).new(5.4, 3.3);
const a1 = clone1(s);
const a2 = clone2(s);
std.debug.warn("{} {}\n", a1.re, a1.im);
std.debug.warn("{} {}\n", a2.re, a2.im);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe more similar is if you use something like the following behaviour which I believe I was using at one point.
fn clone1(z: var) @typeOf(z) {
const T = @typeOf(z.re);
return Complex(T).new(z.im, math.ln(z.re));
}
fn clone2(z: var) Complex(@typeOf(z.re)) {
const s = clone1(z);
return *s;
}
Which gives the following debug output:
-1.43565942e-38 4.59135441e-41
3.29999995 1.68639898
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here the return type of clone1 is &const Complex(f32)
, which is not what we want.
fn clone1(z: var) @typeOf(z) {
const T = @typeOf(z.re);
return Complex(T).new(z.im, z.im);
}
and yeah this is what #733 is intended to solve.
Want to discuss One thing I did is look at LLVM's langref and "complex" is not a concept that exists at the IR level. So that's one reason to have it be in std instead of a primitive. Any other thoughts? |
I think ergonomics is the only thing that is a bit improved being a built-in. However it is far from essential. Given that we don't support user-defined operator overloading there could be slightly more incentive but I don't think that is a strong argument. Let's keep in std for the moment and see if any issues arise later when in use. |
This looked pretty straight-forward so thought I'd get something done. See #947.
An initial implementation using the algorithms specified in musl. This requires more tests for the edge cases but otherwise should be feature complete.
What is the stance on a complex number builtin? I've opted for a library type right now but it was mainly for a placeholder.