Skip to content

feat: add C implementation for stats/base/dists/lognormal/cdf#10883

Closed
rautelaKamal wants to merge 3 commits into
stdlib-js:developfrom
rautelaKamal:feat/stats/base/dists/lognormal/cdf-c-port
Closed

feat: add C implementation for stats/base/dists/lognormal/cdf#10883
rautelaKamal wants to merge 3 commits into
stdlib-js:developfrom
rautelaKamal:feat/stats/base/dists/lognormal/cdf-c-port

Conversation

@rautelaKamal

Copy link
Copy Markdown
Contributor

Description

What is the purpose of this pull request?

This pull request:

  • Adds a C implementation for evaluating the cumulative distribution function (CDF) of a lognormal distribution (@stdlib/stats/base/dists/lognormal/cdf). The implementation delegates to the native stdlib_base_dists_normal_cdf function after log-transforming the input x, adhering to the modular C core approach.

Related Issues

Does this pull request have any related issues?

This pull request has the following related issues:

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

AI Assistance Disclosure

  • Yes
  • No

If yes, how was AI used?
I used an AI to speed up the translation of JS tests to native tests using the Julia fixtures, and to stub out the required C files (addon.c, main.c, build configs). The generated code was then verified against project style guidelines.

Checklist

Please ensure the following tasks are completed before submitting this pull request.

@stdlib-bot stdlib-bot added Statistics Issue or pull request related to statistical functionality. Needs Review A pull request which needs code review. Good First PR A pull request resolving a Good First Issue. labels Mar 11, 2026
@stdlib-bot

Copy link
Copy Markdown
Contributor

Hello! 👋

We've noticed that you've been opening a number of PRs addressing good first issues. Thank you for your interest and enthusiasm!

Now that you've made a few contributions, we suggest no longer working on good first issues. Instead, we encourage you to prioritize cleaning up any PRs which have yet to be merged and then proceed to work on more involved tasks.

Not only does this ensure that other new contributors can work on things and get ramped up on all things stdlib, it also ensures that you can spend your time on more challenging problems. 🚀

For ideas for future PRs, feel free to search the codebase for TODOs and FIXMEs and be sure to check out other open issues on the issue tracker. Cheers!

@stdlib-bot

stdlib-bot commented Mar 11, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

Package Statements Branches Functions Lines
stats/base/dists/lognormal/cdf $\color{green}276/276$
$\color{green}+0.00%$
$\color{green}23/23$
$\color{green}+0.00%$
$\color{green}4/4$
$\color{green}+0.00%$
$\color{green}276/276$
$\color{green}+0.00%$

The above coverage report was generated for the changes in this PR.

@rautelaKamal rautelaKamal marked this pull request as draft March 12, 2026 00:42
@stdlib-bot stdlib-bot removed the Needs Review A pull request which needs code review. label Mar 12, 2026
@rautelaKamal rautelaKamal force-pushed the feat/stats/base/dists/lognormal/cdf-c-port branch from db99cb5 to 31dab56 Compare March 12, 2026 11:06
@rautelaKamal rautelaKamal marked this pull request as ready for review March 12, 2026 15:47
@stdlib-bot stdlib-bot added the Needs Review A pull request which needs code review. label Mar 12, 2026
@rautelaKamal rautelaKamal force-pushed the feat/stats/base/dists/lognormal/cdf-c-port branch from b5f205d to 8fd6468 Compare April 1, 2026 11:13
@github-actions github-actions Bot mentioned this pull request Apr 1, 2026
@rautelaKamal rautelaKamal force-pushed the feat/stats/base/dists/lognormal/cdf-c-port branch from 8fd6468 to 58353a4 Compare April 4, 2026 07:46
@rautelaKamal rautelaKamal force-pushed the feat/stats/base/dists/lognormal/cdf-c-port branch from 58353a4 to 82c5d79 Compare April 12, 2026 07:01
#define NAME "lognormal-cdf"
#define ITERATIONS 1000000
#define REPEATS 3

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've addressed the feedback in the latest push (added Doxygen comments, migrated to the new RNG, and updated isAlmostSameValue usage). This PR should be ready for re-review.

t.strictEqual( y, expected[i], 'x: '+x[i]+', mu:'+mu[i]+', sigma: '+sigma[i]+', y: '+y+', expected: '+expected[i] );
} else {
delta = abs( y - expected[ i ] );
tol = 1500.0 * EPS * abs( expected[ i ] );

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should go ahead and migrate to using isAlmostSameValue for ULP-based difference testing.

@kgryte kgryte changed the title feat: add C implementation for @stdlib/stats/base/dists/lognormal/cdf feat: add C implementation for stats/base/dists/lognormal/cdf Apr 12, 2026
@kgryte kgryte added Needs Changes Pull request which needs changes before being merged. and removed Needs Review A pull request which needs code review. labels Apr 12, 2026
@kgryte

kgryte commented Apr 12, 2026

Copy link
Copy Markdown
Member

@rautelaKamal I updated your PR title. Would you mind updating your other PR titles similarly? This reduces the amount of busy work required from maintainers. Cheers!

@rautelaKamal rautelaKamal force-pushed the feat/stats/base/dists/lognormal/cdf-c-port branch from 82c5d79 to 8b839ee Compare April 13, 2026 07:06
@rautelaKamal rautelaKamal changed the title feat: add C implementation for stats/base/dists/lognormal/cdf feat: add C implementation for stats/base/dists/lognormal/cdf Apr 13, 2026
@rautelaKamal rautelaKamal force-pushed the feat/stats/base/dists/lognormal/cdf-c-port branch from 8b839ee to 5b610f2 Compare April 13, 2026 07:49
@rautelaKamal rautelaKamal force-pushed the feat/stats/base/dists/lognormal/cdf-c-port branch from 5b610f2 to f2b9171 Compare April 13, 2026 08:25
@rautelaKamal

Copy link
Copy Markdown
Contributor Author

I have updated the PR titles as requested and addressed the feedback regarding isAlmostSameValue, Doxygen, and RNG migration. This PR is now ready for re-review.

@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
@Planeshifter

Copy link
Copy Markdown
Member

Thank you for your work on this C implementation, @rautelaKamal, and apologies for the duplicated effort here.

We have two open PRs adding a C implementation for stats/base/dists/lognormal/cdf: this one and #10809. Since only one can land, we're going to move forward with #10809, which was opened a few days earlier (March 8) and has since been kept current. The two implementations are essentially equivalent in their numerical kernel; #10809 additionally aligns its benchmark input ranges with the JS benchmark and migrates the existing JS tests to the isAlmostSameValue convention.

Closing this in favor of #10809. This is no reflection on the quality of your work — it's purely a matter of de-duplicating two parallel efforts. Thanks again for contributing, and we'd welcome your help on other distribution ports.

@stdlib-bot stdlib-bot removed the Needs Review A pull request which needs code review. label Jul 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Good First PR A pull request resolving a Good First Issue. Needs Changes Pull request which needs changes before being merged. Statistics Issue or pull request related to statistical functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants