-
Notifications
You must be signed in to change notification settings - Fork 194
fix: complex dot_product formulation #1017
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
Conversation
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.
Pull Request Overview
This PR fixes the complex dot product formulation in the stdlib intrinsics implementation to ensure compliance with the mathematical standard. The fix addresses issue #1016 by correcting the conjugation logic in complex number multiplication.
- Modifies the complex dot product implementation to conjugate the first vector instead of the second
- Updates both the standard and Kahan summation variants of the dot product functions
- Adds comprehensive test coverage to verify conformance with the compiler intrinsic behavior
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/stdlib_intrinsics_dot_product.fypp | Corrects conjugation order in complex dot product calculations for both standard and Kahan variants |
| test/intrinsics/test_intrinsics.fypp | Adds test case comparing stdlib implementations against compiler intrinsic to ensure standard conformance |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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 good to me.
* fix complex dot_product formulation * smaller array and wider tolerance * Update test/intrinsics/test_intrinsics.fypp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* fix complex dot_product formulation * smaller array and wider tolerance * Update test/intrinsics/test_intrinsics.fypp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* fix complex dot_product formulation * smaller array and wider tolerance * Update test/intrinsics/test_intrinsics.fypp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
For a 32-bit precision dot product, this error is ~0.01, which seems a bit large? I believe using sqrt(epsilon(0.0_${k}$)) would be a more established error threshold in numerical analysis. LGTM otherwise.
addresses #1016
cc: @loiseaujc