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

LWG-2597 std::log misspecified for complex numbers #884

Merged
merged 14 commits into from
Jun 27, 2020

Conversation

statementreply
Copy link
Contributor

@statementreply statementreply commented Jun 8, 2020

  • Fix sqrt, asin, asinh, acos, acosh, atan and atanh of (finite - 0i) and (-0 + finite i).
  • Fix pow(negative - 0i, y).
  • Fix proj(+/-∞ - 0i)
  • Fix sqrt of infinities.

Fix #67.

Use _Abs and _Copysign instead of branching on x < 0, correctly treating negative zero as a negative number. This matters when branch cuts of a complex function lies on the real or the imaginary axis, where the results for positive zero and negative zero are different.

Partially address microsoft#67.

Use `_Abs` and `_Copysign` instead of branching on `x < 0`, correctly
treating negative zero as a negative number. This matters when branch
cuts of a complex function lies on the real or the imaginary axis, where
the results for positive zero and negative zero are different.

The following cases are not fixed in this commit:

* `pow` when the first argument is (negative - 0i).

* (+/-∞ - 0i) and (-0 +/- ∞i).
@ghost
Copy link

ghost commented Jun 8, 2020

CLA assistant check
All CLA requirements met.

@StephanTLavavej StephanTLavavej added the LWG Library Working Group issue label Jun 8, 2020
For negative real number x, pow(x - 0i, y) and pow(x + 0i, y) are
different:

pow(x - 0i, y) = exp(y * log(x - 0i)) = exp(y * (log(|x|) - πi))
pow(x + 0i, y) = exp(y * log(x + 0i)) = exp(y * (log(|x|) + πi))

Thus pow(complex, real) couldn't simply return _Pow(real, real) when the
imaginary part of the base is negative zero. This commit fixes this
issue.

This commit also modifies pow(complex, complex) to be consistent with
pow(real, complex) when the base is a positive real number.
@statementreply statementreply marked this pull request as ready for review June 9, 2020 18:14
@statementreply statementreply requested a review from a team as a code owner June 9, 2020 18:14
Copy link
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

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

Thank you so much for fixing all of these issues - I appreciate the incredibly thorough work. 😸 The product code looks good to me, and I have trivial stylistic suggestions for the test code, which I'll go ahead and push changes for.

tests/std/tests/LWG2597_complex_branch_cut/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/LWG2597_complex_branch_cut/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/LWG2597_complex_branch_cut/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/LWG2597_complex_branch_cut/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/LWG2597_complex_branch_cut/test.cpp Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej removed their assignment Jun 26, 2020
Copy link
Contributor Author

@statementreply statementreply left a comment

Choose a reason for hiding this comment

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

I found a bug in the test code.

tests/std/tests/LWG2597_complex_branch_cut/test.cpp Outdated Show resolved Hide resolved
check_norm(finite, NaN) now fires an assertion as expected.
Copy link
Contributor

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

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

Comment nitpicks, but otherwise this looks great.

stl/inc/complex Outdated Show resolved Hide resolved
stl/inc/complex Outdated Show resolved Hide resolved
@CaseyCarter CaseyCarter self-assigned this Jun 26, 2020
@CaseyCarter CaseyCarter changed the title <complex>: Fix #67: LWG-2597 std::log misspecified for complex numbers LWG-2597 std::log misspecified for complex numbers Jun 26, 2020
* Defend against definition of `signbit` as a function-like macro

* Defend against improper promotions to `double` when computing `log(std::complex<float>)` in `/clr` mode.
stl/inc/complex Outdated Show resolved Hide resolved
@CaseyCarter CaseyCarter merged commit 5b49a88 into microsoft:master Jun 27, 2020
@CaseyCarter
Copy link
Contributor

Thank you for your contribution! I really can't overstate how much we appreciate help from domain experts.

@CaseyCarter CaseyCarter removed their assignment Jun 27, 2020
@statementreply statementreply deleted the fix_complex_branch_cut branch July 18, 2020 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LWG Library Working Group issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LWG-2597 std::log misspecified for complex numbers
5 participants