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

std.complex: Add complex exp(), log(), log10(), and pow() #7456

Merged
merged 4 commits into from
May 9, 2020

Conversation

ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented Apr 25, 2020

Pushing out initial implementation. Unittest incoming...

@ibuclaw ibuclaw added the math label Apr 25, 2020
@ibuclaw ibuclaw requested a review from kyllingstad as a code owner April 25, 2020 15:50
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @ibuclaw!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#7456"

@ibuclaw ibuclaw changed the title std.complex: Add complex exp(), log(), and pow() std.complex: Add complex exp(), log(), log10(), and pow() Apr 25, 2020
std/complex.d Outdated Show resolved Hide resolved
@ibuclaw ibuclaw force-pushed the complexpow branch 2 times, most recently from 84eaf53 to 5eb6608 Compare May 1, 2020 15:52
@ibuclaw
Copy link
Member Author

ibuclaw commented May 1, 2020

Looks like tests are failing because std.complex doesn't have any template constraints for all it's functions. Apparently indirect constraints aren't enough (Complex!T foo(T)() { ... } doesn't inherit constraints from it's return type struct Complex(T) if (isFloatingPoint!T))

@ibuclaw
Copy link
Member Author

ibuclaw commented May 1, 2020

Actually, I can't reproduce with what I thought would be a reduced test, so must be a bug in partial then (I get the same error when replacing pow with cos).

@ibuclaw ibuclaw requested a review from andralex as a code owner May 1, 2020 17:34
@ibuclaw ibuclaw force-pushed the complexpow branch 3 times, most recently from 4b2a457 to b462989 Compare May 2, 2020 15:10
std/complex.d Outdated Show resolved Hide resolved
std/complex.d Show resolved Hide resolved
@ibuclaw ibuclaw force-pushed the complexpow branch 3 times, most recently from c88fb76 to 32b53e2 Compare May 2, 2020 16:14
@ibuclaw ibuclaw added the Review:WIP Work In Progress - not ready for review or pulling label May 2, 2020
@thewilsonator thewilsonator dismissed their stale review May 2, 2020 16:17

Euler is appeased.

@ibuclaw
Copy link
Member Author

ibuclaw commented May 2, 2020

Still WIP are:

  • log() special cases.
  • Put both exp() and log() special cases in Ddoc table.
  • Changelog entry?

Best for another PR...

  • tan is missing, as are also:
  • asin, acos, atan
  • sinh, cosh, tanh
  • norm (squared magnitude)
  • proj (projection onto the Riemann sphere)

@ibuclaw ibuclaw force-pushed the complexpow branch 4 times, most recently from d95e38b to c13a369 Compare May 2, 2020 23:31
@ibuclaw ibuclaw removed Review:WIP Work In Progress - not ready for review or pulling Review:Needs Tests labels May 3, 2020
@ibuclaw ibuclaw force-pushed the complexpow branch 2 times, most recently from 3b525c3 to a5c1b04 Compare May 3, 2020 00:35
std/complex.d Outdated Show resolved Hide resolved
@ibuclaw ibuclaw force-pushed the complexpow branch 4 times, most recently from f37f6ee to 82ea573 Compare May 4, 2020 12:58
@ibuclaw
Copy link
Member Author

ibuclaw commented May 4, 2020

@thewilsonator - green after changing a few more tests from == to approxEqual.

These are a little tedious to write out, would it make sense to add an approxEqual helper for Complex!T?

@PetarKirov
Copy link
Member

would it make sense to add an approxEqual helper for Complex!T?

Or better yet, to isClose (#7384).

@ibuclaw
Copy link
Member Author

ibuclaw commented May 5, 2020

Excellent, I better update all my tests then. :-)

@ibuclaw
Copy link
Member Author

ibuclaw commented May 5, 2020

Excellent, I better update all my tests then. :-)

That can wait for a follow-up PR anyhow, as most tests are isClose, but some only pass with approxEqual.

@ibuclaw
Copy link
Member Author

ibuclaw commented May 7, 2020

@thewilsonator - anything left here?

@dlang-bot dlang-bot merged commit 7b12f8b into dlang:master May 9, 2020
@ibuclaw ibuclaw deleted the complexpow branch May 9, 2020 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants