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

Add double-factorial implementation #1112

Merged
merged 49 commits into from
Nov 17, 2023
Merged

Conversation

rgizz
Copy link
Contributor

@rgizz rgizz commented Oct 23, 2023

Resolves #44 .

Description

What is the purpose of this pull request?

This pull request:

  • adds an implementation of the double factorial function.

Related Issues

Does this pull request have any related issues?

This pull request:

Questions

Any questions for reviewers of this pull request?

Other

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

Prior art:

Checklist

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


@stdlib-js/reviewers

Copy link
Contributor

@stdlib-bot stdlib-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👋 Hi there! 👋

And thank you for opening your first pull request! We will review it shortly. 🏃 💨

@kgryte kgryte added Feature Issue or pull request for adding a new feature. Math Issue or pull request specific to math functionality. Base Issue or pull requests related to "low-level" functionality oriented toward library consumers. labels Oct 24, 2023
@kgryte
Copy link
Member

kgryte commented Oct 24, 2023

As discussed with regard to extension to complex numbers (https://en.wikipedia.org/wiki/Double_factorial#Extensions), the gamma approximation does not agree for even integers. As such, I am inclined to think we should stick with the "classic" definition.

We could, in theory, extend to negative integers; however, I don't think there's an immediate use case, so leaving as restricted to nonnegative integers, seems fine with me.

@kgryte
Copy link
Member

kgryte commented Oct 24, 2023

Additionally, I think that manually re-computing via a loop is fine. A look-up table would likely be a bit faster; however, a static table would significantly increase bundle sizes for what is likely to be a function which is not heavily used.

rgizz added 16 commits October 25, 2023 08:45
This commit adds license to test.js per project requirements.
This commit (1) edits code to conform to style guide; changes 'let'
to 'var', and returns NaN for bad input.
This commit fixes typo in variable name and edits to conform to style guide.
This commit deletes unused decimals code in benchmark.
This commit adds in repl.txt, a standard file not included when
project was started.
To stop git from listing files as untraced in ../factorial directory.
no-verify commit per A. Reines as typescript linter is to be
replaced.
This commit moves README.md from the wrong directory and
corrects some lint errors.
Restore after inadvertently deleting.
This commit adds integers.json, used for testing factorial2. NB these
values are not from Julia but hand calculated.
This file is copied and adapted from factorial directory.
As stated in commit header.
rgizz and others added 25 commits November 14, 2023 16:39
This commit corrects an error as said file should not have
been in this directory in the first place.
Had added .gitignore but not needed all derived from inadvertent
copying of files into wrong directory.
Decimals are removed as the input values are always expected to be
integers, and, similarly, the return values for input values within
range are also expected to be integers.
Copy link
Member

@kgryte kgryte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks, @rgizz!

@kgryte kgryte merged commit c0dab03 into stdlib-js:develop Nov 17, 2023
5 of 7 checks passed
@kgryte kgryte deleted the feat/factorial2 branch November 17, 2023 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Base Issue or pull requests related to "low-level" functionality oriented toward library consumers. Feature Issue or pull request for adding a new feature. Math Issue or pull request specific to math functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement double factorial
3 participants