Add Hyperbolic Trig and atan2#9
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the two-argument arctangent function atan2 for both Computable and Real types, as well as the hyperbolic functions sinh, cosh, and tanh for Real. It also includes corresponding unit tests and micro-benchmarks. The review feedback highlights opportunities to optimize performance by removing redundant .clone() calls in Computable::atan2 and Real::tanh, which will prevent unnecessary heap allocations.
| } | ||
| _ => {} | ||
| } | ||
| let base = self.multiply(x.clone().inverse()).atan(); |
There was a problem hiding this comment.
The parameter x is passed by value to atan2 and is not used after this line. Therefore, calling x.clone() before inverse() is redundant and incurs an unnecessary heap allocation of the Approximation enum. You can directly consume x by calling x.inverse().
let base = self.multiply(x.inverse()).atan();| crate::trace_dispatch!("real", "tanh", "generic-exp-identity"); | ||
| let positive = self.clone().exp()?; | ||
| let negative = self.neg().exp()?; | ||
| (positive.clone() - negative.clone()) / (positive + negative) |
There was a problem hiding this comment.
In the generic path of tanh, positive and negative are cloned unnecessarily. Since Sub and Add are implemented for &Real and Real respectively, we can perform the subtraction by reference (&positive - &negative) and then consume the owned values in the addition (positive + negative). This avoids two expensive heap-allocating clones of Real.
(&positive - &negative) / (positive + negative)
|
Thanks, I’ll make a follow up fixing the review comments |
|
Pretty sure I already got to them |
|
Perfect |
2 participants
This patch adds
sinh,cosh, andtanhto mirror the inverses that are already in the code base. It also addsatan2