stdlib-js / stdlib

✨ Standard library for JavaScript and Node.js. ✨
https://stdlib.io
Apache License 2.0
4.21k stars 412 forks source link

[RFC]: add C implementation for `math/base/special/gammaln` #1988

Open performant23 opened 3 months ago

performant23 commented 3 months ago

Description

This RFC proposes adding native C implementation for @stdlib/math/base/special/gammaln

double stdlib_base_gammaln ( const double x );

Related Issues

Issue Tracker: #649

Questions

No.

Other

No.

Checklist

performant23 commented 3 months ago

Hello, I am working on this issue!

performant23 commented 3 months ago

Hello, I just had a doubt regarding scripts/evalpoly.js. For reference, I checked out this PR and the changes into this file. If I understand correctly, the goal is to extend the functionality to compile polynomial functions in C and insert them into a source file. We're inserting the function string between // BEGIN and // END, creating copts - options object for compileC, read the file and we write the compiled function into the file (using insert).

So basically we're evaluating these polynomials and inserting them into the source files (after compiling). I just wanted to know more on its role and usage on the C implementation and on the library in general since gammaln has a similar script for polynomials.

@kgryte, @Planeshifter

xaman27x commented 3 months ago

@performant23 I have proposed the code and is ready for review. Will release a PR soon! Thank you.

performant23 commented 3 months ago

Hi @xaman27x, I know you're excited about contributing to the repo and the issue but I have completed all the code so far since this was the only issue I was working on since 2 days. If it was a simple implementation, I would have gladly given it but this one was more engaged and demanding than the others generally speaking. I am just waiting for a clarification from the maintainers after which I'll be pushing the code. So I request you to please take on other issues and packages!

kgryte commented 3 months ago

@xaman27x This issue is already being worked on. I suggest finding another issue such as https://github.com/stdlib-js/stdlib/issues/1472.

kgryte commented 3 months ago

@performant23 Your understanding is correct. You need to insert some markup into the C source file so that the script knows where to insert the equations. For generating the polynomial equations, rather than handwrite, we use code generation.

xaman27x commented 3 months ago

@xaman27x This issue is already being worked on. I suggest finding another issue such as #1472. @kgryte Alright no problem Athan! I can see that the repository does not have a C implementation for binomcoef( Binomial Coeffiicient) under math/special . If agreed upon, I can create the corresponding issue and would be glad to work upon this! Or else, would find some another issue.

kgryte commented 3 months ago

@xaman27x You can open an RFC for that package; however, it's implementation is blocked, as binomcoef requires the following PR to be merged: https://github.com/stdlib-js/stdlib/pull/1703.

xaman27x commented 3 months ago

@xaman27x You can open an RFC for that package; however, it's implementation is blocked, as binomcoef requires the following PR to be merged: #1703.

@kgryte Alright fine, I'll look into the same. Also I would highly appreciate if you have the reference of any C implementation of mathematical functions that is yet to be resolved and has a high priority, since many of them already are being worked on or have existing PR's. I will love to work upon them. Thank You! Best.

kgryte commented 3 months ago

@xaman27x See my comment above: https://github.com/stdlib-js/stdlib/issues/1988#issuecomment-2016700975

performant23 commented 3 months ago

Hi @kgryte, I request you to temporarily block this issue. I've just discovered an overlooked dependency in the C implementation of special/sinpi, which also relies on special/sin and special/cos. Apologies for not noticing this sooner. Realized this since I encountered an error during compilation and execution of the package.

kgryte commented 3 months ago

@performant23 Thanks for the update. Will update the status.