-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Complex numbers #3892
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
base: master
Are you sure you want to change the base?
Complex numbers #3892
Conversation
|
Labeling this T-lang because the desire to make this FFI-compatible is a lang matter. |
text/3892-complex-numbers.md
Outdated
| ```rust | ||
| let y = Complex::from_array([3, 4]); | ||
| ``` | ||
| or as a tuple: | ||
| ```rust | ||
| let z = Complex::from_tuple((3, 4)); | ||
| ``` |
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.
i think impl<T> From<(T, T)> for Complex<T> and impl<T> From<[T; 2]> for Complex<T> would be sufficient. it's like there is no SocketAddr::from_tuple.
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.
This still exists. @scimind2460 please make sure you adequately address review concerns before marking topics as resolved, or leave them open for the the commenter to mark resolved themselves. If you're not sure, leave a comment.
|
It's worth pointing out another big issue with this is that the canonical In particular, while I'm not super compelled by the argument that C supports this, therefore the standard library needs to support this. I think that guaranteeing a |
I think that polar form almost always is the more optimal form, at least in my experience. But the ABIs do use rectangular, e.g. from x86:
so it makes sense that an interchange type matches that, and users can translate to/from a polar repr at the FFI boundary if needed. But this reasoning is definitely something to have in the RFC's rationale & alternatives. |
|
Right: I guess my main clarification here was that due to the polar-orthogonal discrepancy, it shouldn't be a canonical Rust type (e.g. |
|
Thanks everyone for the feedback! I have incorporated as much as I can into the RFC. |
text/3892-complex-numbers.md
Outdated
| impl Complex<f64> { | ||
| fn angle(self) { | ||
| f32::atan2(self.re(), self.im()) | ||
| } | ||
| fn from_polar(modulus: f32, angle: f32) -> Complex<f32> { | ||
| Complex::new(modulus * f32::cos(angle), modulus * f32::sin(angle)) | ||
| } | ||
| } |
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.
Probably want to return something here? And not use the f32 types for f64
That being said: I think polar conversions should be put into "future possibilities" since they aren't needed for basic support.
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.
I was trying to write something to help with @clarfonthey's polar proposal, but if they are fine with this becoming a future possibility, then OK!
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.
Agreed that they should be moved to "future possibilities" for now.
Refactor Complex struct to have public fields and update methods for real and imaginary parts. Added notes on future considerations for polar conversions and arithmetic implementations.
Co-authored-by: Josh Triplett <[email protected]>
cuviper
left a comment
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.
I apologise in advance to
num-complex
No worries! There are definitely things that a compiler-integrated implementation can do better, especially with ABI as you've highlighted. A lot of the num-complex API is also limited by trying to be so generic, whereas the standard library can choose to keep all the implementation flexibility for itself.
Co-authored-by: Jacob Lifshay <[email protected]>
text/3892-complex-numbers.md
Outdated
| ```rust | ||
| let y = Complex::from_array([3, 4]); | ||
| ``` | ||
| or as a tuple: | ||
| ```rust | ||
| let z = Complex::from_tuple((3, 4)); | ||
| ``` |
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.
This still exists. @scimind2460 please make sure you adequately address review concerns before marking topics as resolved, or leave them open for the the commenter to mark resolved themselves. If you're not sure, leave a comment.
|
I'd love to see an FCP kicked off for this, as soon as the open comments are addressed on the RFC (extra functions that need to move to future possibilities, and the inaccurate background information on nalgebra). |
Co-authored-by: Trevor Gross <[email protected]>
Co-authored-by: Trevor Gross <[email protected]>
Removed redundant statement about overhead from libgcc calls and clarified the purpose of the Complex type.
|
Hopefully I've cleared up everything to satisfaction. I suppose it's broadly reasonable to FCP? |
|
The current state of the RFC looks good, thank you! This continues to be a lang/libs-api RFC, lang because of the FFI interoperability with C @rfcbot merge libs-api,lang |
|
Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
Co-authored-by: kennytm <[email protected]>
Clarify the rationale behind complex number implementation alternatives.
Expanded on the limitations of polar complex numbers and the need for a standard type for FFI with C. Discussed alternatives to complex number representation and their implications.
Added a section discussing alternatives to complex number implementations, including polar layout and non-generic primitive types, while emphasizing the importance of FFI compatibility.
This RFC proposes FFI-compatible complex numbers to help scientific computing library authors use non-indirected complexes.
I apologise in advance to
num-complexRendered