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.

Sign up for GitHub

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

Jump to bottom

[RFC]: Add C implementation for @stdlib/math/base/special/negafibonacci #1758

Closed
3 tasks done
gunjjoshi opened this issue Mar 7, 2024 · 4 comments · Fixed by #1813
Closed
3 tasks done

[RFC]: Add C implementation for @stdlib/math/base/special/negafibonacci #1758

gunjjoshi opened this issue Mar 7, 2024 · 4 comments · Fixed by #1813
Assignees
@gunjjoshi
Labels
Accepted RFC feature request which has been accepted. C Issue involves or relates to C. difficulty: 2 May require some initial design or R&D, but should be straightforward to resolve and/or implement. Feature Issue or pull request for adding a new feature. Good First Issue A good first issue for new contributors! Math Issue or pull request specific to math functionality. priority: Normal Normal priority concern or feature request. RFC Request for comments. Feature requests and proposed changes.

Comments

@gunjjoshi
Copy link
Member

Description

This RFC proposes adding C implementation for @stdlib/math/base/special/negafibonacci.

Related Issues

Related issues #649

Questions

No.

Other

No.

Checklist

  • I have read and understood the Code of Conduct.
  • Searched for existing issues and pull requests.
  • The issue name begins with RFC:.
@gunjjoshi
Copy link
Member Author

I would like to work on this issue.

@kgryte
Copy link
Member

kgryte commented Mar 7, 2024

Thanks for posting this RFC, @gunjjoshi. Yes, this works, so long as we have all the requisite C dependencies implemented.

@kgryte kgryte added RFC Request for comments. Feature requests and proposed changes. Feature Issue or pull request for adding a new feature. Math Issue or pull request specific to math functionality. Accepted RFC feature request which has been accepted. Good First Issue A good first issue for new contributors! priority: Normal Normal priority concern or feature request. C Issue involves or relates to C. difficulty: 2 May require some initial design or R&D, but should be straightforward to resolve and/or implement. labels Mar 7, 2024
@gunjjoshi
Copy link
Member Author

Hey @kgryte, while working on this, I found that even in the C implementation of @stdlib/math/base/special/fibonacci, there is no check for identifying if the given number is an integer or not, or the given number is NaN or not. But in its Javascript implementation, those checks are being done.

As a result, the last 2 examples in its native.js file give error messages, indicating that the expected values are something else, and not NaN.

Or else, am I getting wrong somewhere ?

@kgryte
Copy link
Member

kgryte commented Mar 9, 2024

@gunjjoshi I think you are right. For fibonacci, we should remove the non-integer, negative integer, and NaN input examples for the native.js file. That was a mistake and should be addressed in a separate PR.

This was referenced Mar 10, 2024
Merged
Merged
Planeshifter added a commit that referenced this issue May 29, 2024
@gunjjoshi @Pranavchiku @Planeshifter
feat: add C implementation for math/base/special/negafibonacci
1bc3682 
PR-URL: #1813
Closes: #1758 
Ref: #649

---------

Signed-off-by: GUNJ JOSHI <[email protected]>
Signed-off-by: Pranav Goswami <[email protected]>
Co-authored-by: Pranav Goswami <[email protected]>
Co-authored-by: Philipp Burckhardt <[email protected]>
Reviewed-by: Philipp Burckhardt <[email protected]> 
Reviewed-by: Athan Reines <[email protected]> 
Reviewed-by: Pranav Goswami <[email protected]>
aman-095 pushed a commit to aman-095/stdlib that referenced this issue Jun 13, 2024
@gunjjoshi @Pranavchiku
@Planeshifter @aman-095
feat: add C implementation for math/base/special/negafibonacci
1a4dffc 
PR-URL: stdlib-js#1813
Closes: stdlib-js#1758 
Ref: stdlib-js#649

---------

Signed-off-by: GUNJ JOSHI <[email protected]>
Signed-off-by: Pranav Goswami <[email protected]>
Co-authored-by: Pranav Goswami <[email protected]>
Co-authored-by: Philipp Burckhardt <[email protected]>
Reviewed-by: Philipp Burckhardt <[email protected]> 
Reviewed-by: Athan Reines <[email protected]> 
Reviewed-by: Pranav Goswami <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gunjjoshi gunjjoshi

Labels
Accepted RFC feature request which has been accepted. C Issue involves or relates to C. difficulty: 2 May require some initial design or R&D, but should be straightforward to resolve and/or implement. Feature Issue or pull request for adding a new feature. Good First Issue A good first issue for new contributors! Math Issue or pull request specific to math functionality. priority: Normal Normal priority concern or feature request. RFC Request for comments. Feature requests and proposed changes.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: add C implementation for math/base/special/negafibonacci
2 participants
@kgryte @gunjjoshi