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

Changed the tanh derivative from g/cosh(x)**2 to g*(1-tanh(x)**2) #319

Merged
merged 1 commit into from
Oct 21, 2017

Conversation

RmStorm
Copy link
Contributor

@RmStorm RmStorm commented Oct 20, 2017

This way ans can be reused as in g*(1-ans**2) which should be a little quicker since it omits the cosh call. But mainly I get overflow warnings because of the use of cosh. This new quicker derivative solves that.

…s way ans can be reused as in g*(1-ans**2). But mainly I get overflow warnings because of the use of cosh. This new quicker derivative solves that.
@j-towns
Copy link
Collaborator

j-towns commented Oct 21, 2017

👍 I found this quite a lot quicker and hav included it in #312, but that probably won't be merged into master very soon so this pr is good in the mean time. You can do something similar for the derivative of tan and of sinh.

@j-towns
Copy link
Collaborator

j-towns commented Oct 21, 2017

It's tantalising for derivatives of sin and cos too because calculating sqrt(1 - x**2) is way quicker than calculating sin/cos (around 8x faster on my laptop), but you need to also calculate the correct sign, which slows things back down, at least in my implementation attempts.

@dougalm dougalm merged commit b3eb51d into HIPS:master Oct 21, 2017
@mattjj
Copy link
Contributor

mattjj commented Oct 21, 2017

This is great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants