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

[RFC]: Add C implementation for @stdlib/math/base/special/fibonacci-index #1756

Closed
3 tasks done
tudor-pagu opened this issue Mar 7, 2024 · 7 comments
Closed
3 tasks done
Assignees
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

@tudor-pagu
Copy link
Contributor

tudor-pagu commented Mar 7, 2024

Description

This RFC proposes adding native C implementation for @stdlib/math/base/special/fibonacci-index.

double stdlib_base_fibonacci_index( const double F );

Related Issues

Tracked in issue #649.

Questions

I am not sure about the method's signature. In the original JavaScript code, both the return type and the parameter are {NonNegativeInteger}. However, in some tests they take on the value NaN, which as far as I know does not exist for integer types in C. For this reason, I decided on using double, but please tell me if there is a more suitable solution.

Other

The implementation depends on the round function, which is addressed in #735. I can use the round method from C until the method is implemented in stdlib, and I can also contribute to implementing round.

Checklist

  • I have read and understood the Code of Conduct.
  • Searched for existing issues and pull requests.
  • The issue name begins with RFC:.
@stdlib-bot
Copy link
Contributor

👋 Hi there! 👋

And thank you for opening your first issue! We will get back to you shortly. 🏃 💨

@tudor-pagu
Copy link
Contributor Author

Hi @kgryte @Planeshifter @Pranavchiku, I would like to work on this issue, could it please be assigned to me?

@kgryte
Copy link
Member

kgryte commented Mar 7, 2024

@tudor-pagu We cannot use C round as it's behavior diverges from JS's behavior. This RFC is blocked until we can get #735 over the finish line.

@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! status: Blocked Issue or pull request which is current blocked. 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. priority: Normal Normal priority concern or feature request. labels Mar 7, 2024
@vivek-anand-singh
Copy link

Since #735 issue is closed now. Can you please assign this to me @kgryte @Pranavchiku

@kgryte
Copy link
Member

kgryte commented Jul 26, 2024

@vivek-anand-singh Yes, assigned. Thanks for volunteering to work on this.

@gunjjoshi
Copy link
Member

@kgryte We can close this issue, as it is completed by #2272.

@kgryte
Copy link
Member

kgryte commented Jul 27, 2024

Thanks, @gunjjoshi, for following up here.

@kgryte kgryte closed this as completed Jul 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging a pull request may close this issue.

5 participants