Skip to content
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

complex numbers #481

Merged
merged 1 commit into from
Nov 30, 2023
Merged

complex numbers #481

merged 1 commit into from
Nov 30, 2023

Conversation

chachaleo
Copy link
Contributor

Pull Request type

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build-related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Issue Number: #464

What is the new behavior?

I implemented the complex numbers, in a similarly to fixed point and signed integer.
I only implemented for FP64x64 for now but I can implement the ComplexTrait for other precision.

As a lot of functions implemented in the ComplexTrait use some not completely precise functions (Fixed point computations : sqrt, cos, ln,...) they don't return the exact results. I added comments in each tests with the expected result and the actual result so that you can have an idea of the precision and judge if the precision is good enough. If it's not good enough I'd love to investigate what we could do about it.

Also I can add additional complex analysis functions if needed.

I plan on also implement the corresponding Tensor but I wanted a feedback on this first to be sure I am in the right direction :)

Copy link
Collaborator

@raphaelDkhn raphaelDkhn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Incredible work @chachaleo! The precision is good enough imo

@raphaelDkhn raphaelDkhn merged commit 0b92daa into gizatechxyz:develop Nov 30, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants