Add std.complex.coshisinh and remove uses of creal math. #5698
Conversation
|
Thanks for your pull request, @ibuclaw! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. |
std/complex.d
Outdated
| */ | ||
| Complex!real coshisinh(real y) @safe pure nothrow @nogc | ||
| { | ||
| import std.math : cosh, sinh; |
There was a problem hiding this comment.
Is this the most efficient way of performing the calculation? I'm only asking because the one in std.math is implemented differently, using exp()/expm1().
There was a problem hiding this comment.
std.math.coshisinh just inlines the cosh+sinh implementations.
There was a problem hiding this comment.
Yes, but the purpose of that seems to be to reduce two exp() calls to one. Does the compiler perform the same optimisation in your case?
There was a problem hiding this comment.
Two calls would be made. One for cosh, one for sinh.
There was a problem hiding this comment.
std.math.coshisinh makes only one exp call. In fact, that seems to be its raison d'être.
|
Added one fast path. I also noticed there could be a few more functions added here. log, exp and tan are some of the notable that should be easy to add, but missing. |
@ibuclaw do you intend to add them in this PR, or is it now good to go? |
|
I can add other functions in latter PRs. I should alzo add another unittest for |
| } | ||
| } | ||
|
|
||
| @safe pure nothrow @nogc unittest |
There was a problem hiding this comment.
How about making this a publicly documented example?
| Note: | ||
| $(D coshisinh) is included here for convenience and for easy migration of code | ||
| that uses $(REF _coshisinh, std,math). |
There was a problem hiding this comment.
What is the motivation behind the move?
There was a problem hiding this comment.
Are we deprecating complex real?
| static import std.math; | ||
| if (std.math.fabs(y) <= 0.5) | ||
| return Complex!real(std.math.cosh(y), std.math.sinh(y)); | ||
| else |
dlang-bot
added
Merge:stalled
Merge:Needs Rebase
and removed
Merge:stalled
Merge:Needs Rebase
labels
andralex
left a comment
andralex
left a comment
There was a problem hiding this comment.
This shouldn't be blocked on me as I'm not an expert.
|
cc math people @9il @klickverbot @WalterBright |
7 participants
Small addition for dlang/dmd#7081
Everything else looks like just deleting support code from phobos.