Skip to content

feat: add C implementation for stats/base/dists/erlang/logpdf#10719

Open
rautelaKamal wants to merge 9 commits into
stdlib-js:developfrom
rautelaKamal:feat/stats/base/dists/erlang/logpdf-c-port
Open

feat: add C implementation for stats/base/dists/erlang/logpdf#10719
rautelaKamal wants to merge 9 commits into
stdlib-js:developfrom
rautelaKamal:feat/stats/base/dists/erlang/logpdf-c-port

Conversation

@rautelaKamal
Copy link
Copy Markdown
Contributor

@rautelaKamal rautelaKamal commented Mar 5, 2026 •
edited
Loading

This PR adds a C implementation for @stdlib/stats/base/dists/erlang/logpdf.

Closes #3571

Changes

  • Added src/addon.c: Node.js native addon entry point

    • Added src/main.c: C implementation delegating to stdlib_base_dists_gamma_logpdf
    • Added include/stdlib/stats/base/dists/erlang/logpdf.h: C header file
    • Added lib/native.js: JavaScript wrapper for native addon
    • Added benchmark/benchmark.native.js: Native benchmark
    • Added manifest.json: Build manifest
    • Updated package.json: Added gypfile, include, and src directories

  • Read, understood, and followed the contributing guidelines

AI Assistance

  • I have used AI-assisted tools for some of the changes in this PR.

@stdlib-bot stdlib-bot added the Statistics Issue or pull request related to statistical functionality. label Mar 5, 2026
@stdlib-bot
Copy link
Copy Markdown
Contributor

stdlib-bot commented Mar 5, 2026

Hello! Thank you for your contribution to stdlib.

We noticed that the contributing guidelines acknowledgment is missing from your pull request. Here's what you need to do:

  1. Please read our contributing guidelines.

  2. Update your pull request description to include this checked box:

    - [x] Read, understood, and followed the [contributing guidelines](https://github.com/stdlib-js/stdlib/blob/develop/CONTRIBUTING.md)

This acknowledgment confirms that you've read the guidelines, which include:

  • The developer's certificate of origin
  • Your agreement to license your contributions under the project's terms

We can't review or accept contributions without this acknowledgment.

Thank you for your understanding and cooperation. We look forward to reviewing your contribution!

@stdlib-bot stdlib-bot added Needs Review A pull request which needs code review. Good First PR A pull request resolving a Good First Issue. labels Mar 5, 2026
@stdlib-bot
Copy link
Copy Markdown
Contributor

stdlib-bot commented Mar 5, 2026 •
edited
Loading

Coverage Report

No coverage information available.

This was referenced Mar 30, 2026
Closed
Closed
@rautelaKamal rautelaKamal force-pushed the feat/stats/base/dists/erlang/logpdf-c-port branch 5 times, most recently from 55dc671 to b76bc1a Compare April 4, 2026 18:08
@github-actions github-actions Bot mentioned this pull request Apr 4, 2026
Closed
@rautelaKamal rautelaKamal force-pushed the feat/stats/base/dists/erlang/logpdf-c-port branch 4 times, most recently from 2b8636f to 93de059 Compare April 12, 2026 07:00
@kgryte kgryte added difficulty: 2 May require some initial design or R&D, but should be straightforward to resolve and/or implement. review: 5 and removed Needs Review A pull request which needs code review. labels Apr 12, 2026
@kgryte kgryte changed the title feat: add C implementation for @stdlib/stats/base/dists/erlang/logpdf feat: add C implementation for stats/base/dists/erlang/logpdf Apr 12, 2026
kgryte
kgryte reviewed Apr 12, 2026
View reviewed changes
Comment thread lib/node_modules/@stdlib/stats/base/dists/erlang/logpdf/benchmark/benchmark.native.js Outdated
kgryte
kgryte reviewed Apr 12, 2026
View reviewed changes
Comment thread lib/node_modules/@stdlib/stats/base/dists/erlang/logpdf/test/test.native.js Outdated
@kgryte kgryte added review: 4 Needs Changes Pull request which needs changes before being merged. and removed review: 5 labels Apr 12, 2026
@rautelaKamal rautelaKamal force-pushed the feat/stats/base/dists/erlang/logpdf-c-port branch from 93de059 to 7a15449 Compare April 13, 2026 07:08
@rautelaKamal rautelaKamal changed the title feat: add C implementation for stats/base/dists/erlang/logpdf feat: add C implementation for stats/base/dists/erlang/logpdf Apr 13, 2026
@rautelaKamal rautelaKamal force-pushed the feat/stats/base/dists/erlang/logpdf-c-port branch from 7a15449 to 54bf6dd Compare April 13, 2026 07:51
@rautelaKamal rautelaKamal requested a review from a team April 20, 2026 13:14
@stdlib-bot stdlib-bot added the Needs Review A pull request which needs code review. label Apr 20, 2026
@rautelaKamal rautelaKamal requested a review from kgryte April 20, 2026 13:29
@Neerajpathak07 Neerajpathak07 changed the title feat: add C implementation for stats/base/dists/erlang/logpdf feat: add C implementation for stats/base/dists/erlang/logpdf Jun 3, 2026
@Neerajpathak07
Copy link
Copy Markdown
Member

Neerajpathak07 commented Jun 3, 2026

Note:- For this PR I went ahead and modernized the existing JS benchmarks and tests to follow the latest code conventions.

Neerajpathak07
Neerajpathak07 reviewed Jun 3, 2026
View reviewed changes
Comment thread lib/node_modules/@stdlib/stats/base/dists/erlang/logpdf/src/main.c
if ( !stdlib_base_is_nonnegative_integer( k ) ) {
return 0.0 / 0.0; // NaN
}
return stdlib_base_dists_gamma_logpdf( x, k, lambda );
Copy link
Copy Markdown
Member

@Neerajpathak07 Neerajpathak07 Jun 3, 2026

Choose a reason for hiding this comment

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

since gamma/logpdf accurately handles edge cases for the 2nd and 3rd argument which is k and lambda in this case. For ex:- lambda <= 0.0 should return NaN.

I removed the previous unwanted checks to validate if k is NaN and sp on!!

Neerajpathak07
Neerajpathak07 reviewed Jun 3, 2026
View reviewed changes
Comment thread lib/node_modules/@stdlib/stats/base/dists/erlang/logpdf/test/test.logpdf.js
delta = abs( y - expected[ i ] );
tol = 450.0 * EPS * abs( expected[ i ] );
t.ok( delta <= tol, 'within tolerance. x: '+x[ i ]+'. k: '+k[i]+'. lambda: '+lambda[i]+'. y: '+y+'. E: '+expected[ i ]+'. Δ: '+delta+'. tol: '+tol+'.' );
t.ok( isAlmostSameValue( y, expected[i], 700 ), 'within tolerance. x: '+x[ i ]+'. k: '+k[i]+'. lambda: '+lambda[i]+'. y: '+y+'. E: '+expected[ i ]+'.' );
Copy link
Copy Markdown
Member

@Neerajpathak07 Neerajpathak07 Jun 3, 2026

Choose a reason for hiding this comment

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

These test cases are expected to have huge-diffs due to the large values of generated fixtures. Something, which was previously done for other packages in math namespace.

@Neerajpathak07
Copy link
Copy Markdown
Member

Neerajpathak07 commented Jun 3, 2026

Since the lint faliure is not due to changes associated with this PR I'll go ahead and request up a review!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Neerajpathak07 Neerajpathak07 Neerajpathak07 left review comments

@kgryte kgryte Awaiting requested review from kgryte

@Planeshifter Planeshifter Awaiting requested review from Planeshifter

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

difficulty: 2 May require some initial design or R&D, but should be straightforward to resolve and/or implement. Good First PR A pull request resolving a Good First Issue. Needs Changes Pull request which needs changes before being merged. Needs Review A pull request which needs code review. review: 4 Statistics Issue or pull request related to statistical functionality.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[RFC]: Add C implementation for @stdlib/stats/base/dists/erlang/logpdf

4 participants

@rautelaKamal @stdlib-bot @Neerajpathak07 @kgryte