Add support for computing the inverse hyperbolic cotangent (acoth)

[BrunoFenzl]

Resolves #236.

Checklist

Please ensure the following tasks are completed before submitting this pull request.

• Read, understood, and followed the contributing guidelines, including the relevant style guides.
• Read and understand the Code of Conduct.
• Read and understood the licensing terms.
• Searched for existing issues and pull requests before submitting this pull request.
• Filed an issue (or an issue already existed) prior to submitting this pull request.
• Rebased onto latest develop.
• Submitted against develop branch.

Description

What is the purpose of this pull request?

This pull request:

• implements issue RFC: add support for computing the inverse hyperbolic cotangent (acoth) #236

Related Issues

Does this pull request have any related issues?
no

This pull request:

• resolves RFC: add support for computing the inverse hyperbolic cotangent (acoth) #236

Questions

Any questions for reviewers of this pull request?
tex notation docs are not up to date. I have to read my way through to update them.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

Happy to have some feedback!

---

@stdlib-js/reviewers

[kgryte]

@BrunoFenzl Thanks for working on this! This PR is useful for us, as it highlights the need for us to clarify what files are needed when, particularly when it comes to testing and benchmarking math implementations. For example, several of the files which are part of acosh are not applicable here. Sorry about this! Thanks for your patience and perseverance!

[kgryte]

Suggested change

	The domain of `x` is restricted to `[x < -1 || x > 1)`. If `x >= -1 || <= 1`, the function will return `NaN`.
	If `x` is on the interval `[-1, 1]`, the function returns `NaN`.

[BrunoFenzl]

On a second look, if x is exactly -1 or 1, this implementation it will return 0. NaN will be returned if x is less than 1 and more than -1 but not equal. This was my mistake.

[kgryte]

This can be removed, along with the "built-in" benchmark. The built-in Math object does not have a acoth method, nor are there any proposals to add to the language.

[kgryte]

Suggested change

	x = ( randu()*100.0 ) + 1.0;
	x = ( randu()*100.0 ) + 1.1;

randu() returns values on the interval [0,1). In which case, need to add a value slightly greater than 1 in order to ensure that y is not NaN.

[kgryte]

Suggested change

	x = linspace( 3.0, 28.0, 500 );
	x = linspace( 3.0, 28.0, 503 );

[kgryte]

Suggested change

	x = linspace( 28.0, 100.0, 500 );
	x = linspace( 28.0, 100.0, 503 );

[kgryte]

Suggested change

	x = linspace( 1e300, 1e308, 1001 );
	x = linspace( 1e300, 1e308, 1003 );

[kgryte]

Suggested change

	tape( 'the function returns `NaN` if provided value between `-1` and `1`', function test( t ) {
	tape( 'the function returns `NaN` if provided a value between `-1` and `1`', function test( t ) {

Inclusive or exclusive? Not clear. The docs seem to suggest inclusive, but various example output suggests otherwise.

[BrunoFenzl]

Should be exclusive.

[kgryte]

Suggested change

	v = -(randu()*1.0e6) + (1-EPS);
	v = -(randu()*1.0e6) + (1.0-EPS);

We include decimals for code documentation purposes when a value and/or operations involve real-valued numbers. If a value can only be integer-valued, project convention is to omit the decimal.

[BrunoFenzl]

ah! That's why... I was going to ask you why those units were being written with decimal points. Thanks for the clarification.

[BrunoFenzl]

Wow! Thanks for the thoroughly review and sorry for the tons of inconsistencies! I'll take more care on the other functions I did, specially concerning the implementation in other languages. I have never seen such a complete testing suite (catching docs inconsistencies is awesome btw.) so I used the acosh folder as a template trying not to miss anything. I really hope I didn't take too much time from you.

[kgryte]

@BrunoFenzl No worries! This is part of the process. The reality is that getting up-to-speed with all the project conventions in addition to implementing numerical algorithms can be a dizzying task (something the project needs to do a better job of addressing by better on-boarding and documentation). So thanks for putting in all the work!

[BrunoFenzl]

@kgryte I think this PR is good now. Please let me know what I could improve if you find something wrong.

[kgryte]

@BrunoFenzl Awesome! Will review shortly... :)

[kgryte]

Why is this special case present? The inverse hyperbolic cotangent is not defined for 0.0.

[BrunoFenzl]

atanh should return 0.0 but is returning NaN. I probably missed something, but I could not figure it out why.

[kgryte]

atanh does return 0.0. To see this in action, start the stdlib REPL,

$ make repl

In the REPL,

> base.atanh( 0.0 )
0.0

For the inverse hyperbolic cotangent acoth, the equivalent atanh call is

> base.atanh( 1.0 / 0.0 )
NaN

This result is the same in Julia, Python, etc.

That the result should be NaN and not 0.0 can be confirmed by examining the plot of acoth where the domain is not defined in the region between (-1,1), as also stated in this package's documentation.

Hope this helps! :)

[kgryte]

This file still needs to be removed.

[kgryte]

@BrunoFenzl Did a quick review. Most of the remaining changes I flagged concern files which are not applicable to acoth.

[kgryte]

@BrunoFenzl Just for reference, looking at the PR commit log, instead of merging from your fork into what I assume to be the cloned project repository, you want to merge from the project remote (i.e., this repo) into your fork. This will help prevent mixing your contributions with updates from the upstream repository (e.g., this PR indicates that 119 files have changed, when this PR really only concerns about 20 files). For example, assuming you have forked the project and cloned the fork to your local machine,

$ cd ./path/to/your/fork/stdlib
$ git checkout <your_feature_branch>
$ git remote -v
$ git remote add upstream https://github.com/stdlib-js/stdlib.git
$ git remote -v
$ git fetch upstream
$ git merge upstream/develop

I believe this is the workflow for syncing a feature development branch (e.g., feature/acoth) with changes in this repository.

Apologies if this is what you did! Something just seems a bit off with the commit log.

[kgryte]

Ah...wait...did you commit directly to a develop branch in your fork? That seems to be why. Would you mind porting your changes to a feature/* branch (see also this guide)? And then reopening a new PR from that feature branch to develop on this repository?

Sorry about this! I should have caught this earlier! In short, because this PR is attempting to go from develop to develop, the PR's commits are being interleaved with changes in the upstream repository, which will be problematic when I "squash-and-merge" this PR.

[BrunoFenzl]

Oh! Sorry for the silly mistake! I did fetch upstream to synch with develop and forgot to switch back to my feature branch. I'll make a new pull request from a clean branch. BTW, I got rid of those errors in julia with version 0.5.2. It was no issue at all downgrading it. Since I'm on a mac, I just deleted the app, removed the preferences and installed again. Sorry for the extra work!

[kgryte]

@BrunoFenzl No worries! We need to do a better job of making contributing a smoother process. Really appreciate your persistence and continued work!