Fix factorial implementation #17
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
thomwiggers
approved these changes
Nov 23, 2023
| + Clone | ||
| + FromPrimitive | ||
| + ToPrimitive | ||
| + Shl<u32, Output = T>, |
There was a problem hiding this comment.
This changes the API, so I suppose that we have to bump to 0.4.0 now.
|
I would also like to note that the blame rests as much on my shoulders, I really should have tested this better. Fortunately, we learn as we go and the unit tests are now a lot better. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
First of all, I'm really sorry for the faulty implementation, but this fix it. See #16
There's no performance degradation with this PR is it essentially the same, but now the algorithm always use prime swing because the implementation is a bit more efficient for small numbers. (Before it was using the naive algorithm to calculate number below 1200)
