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

feat: add C implementation for @stdlib/stats/base/dists/lognormal/logcdf #4716

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

saurabhraghuvanshii
Copy link
Contributor

Resolves #3750 .

Description

What is the purpose of this pull request?

This pull request:

  • Add C implementation for @stdlib/stats/base/dists/lognormal/logcdf

Related Issues

Does this pull request have any related issues?

This pull request:

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.

test case failed , I try multiple things.

Checklist

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


@stdlib-js/reviewers

@stdlib-bot stdlib-bot added Statistics Issue or pull request related to statistical functionality. Needs Review A pull request which needs code review. labels Jan 12, 2025
@saurabhraghuvanshii
Copy link
Contributor Author

some errors are showing in readme section, And I don't know to tackle with that, I need help and review and Tests are failed as I done every type of possible solution by my side .

@saurabhraghuvanshii
Copy link
Contributor Author

@Planeshifter @kgryte review this , I have some doubt with this.

* double y = stdlib_base_lognormal_logcdf( 2.0, 0.0, 1.0 );
* // returns ~-0.2799
*/
double stdlib_base_dists_lognormal_logcdf( const double x, const double mu, const double sigma ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@saurabhraghuvanshii The test cases are throwing errors because your implementation for main.c file does not resonate with the lib/main.js
According to the lib/main.js you'll be needing C implementations for nlogcdf

Copy link
Contributor Author

@saurabhraghuvanshii saurabhraghuvanshii Jan 12, 2025

Choose a reason for hiding this comment

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

means return NINF / ln(x), if yes then how import ln it show there is no directory

stdlib_base_is_nan( sigma ) ||
sigma <= 0.0
) {
return 0.0 / 0.0; // NaN
Copy link
Contributor

@Neerajpathak07 Neerajpathak07 Jan 12, 2025

Choose a reason for hiding this comment

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

it should be returning NINF when true and

ln(x)

while false

Copy link
Contributor Author

@saurabhraghuvanshii saurabhraghuvanshii Jan 12, 2025

Choose a reason for hiding this comment

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

`#include "stdlib/stats/base/dists/lognormal/logcdf.h"
#include "stdlib/math/base/special/ln.h"
#include "stdlib/constants/float64/ninf.h"

/**

  • Returns the logcdf for a lognormal distribution with mean mu and standard deviation sigma.
  • @param x input value
  • @param mu mean
  • @param sigma standard deviation
  • @return logcdf
  • @example
  • double y = stdlib_base_lognormal_logcdf( 2.0, 0.0, 1.0 );
  • // returns ~-0.2799
    */
    double stdlib_base_dists_lognormal_logcdf( const double x, const double mu, const double sigma ) {
    double lx = ( x <= 0.0 ) ? ninf : ln( x );
    return nlogcdf( lx, mu, sigma );
    }
    `
    error on building

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fatal error: stdlib/math/base/special/ln.h: No such file or directory

Copy link
Contributor

@Neerajpathak07 Neerajpathak07 Jan 12, 2025

Choose a reason for hiding this comment

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

did you add those header files in manifest.json also that's not how you write ninf it should be STDLIB_CONSTANT_FLOAT64_NINF

Copy link
Contributor

@Neerajpathak07 Neerajpathak07 Jan 12, 2025

Choose a reason for hiding this comment

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

Similarly instead of ln(x) write stdlib_base_ln(x)
same goes for nlogcdf

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same error happen

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can you go through files annd check why all this happens

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am comiting with new changes check it .

Copy link
Contributor

Choose a reason for hiding this comment

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

sure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

now you can check,

@saurabhraghuvanshii
Copy link
Contributor Author

I have to clean up , I don't how all these commit comes from

---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: na
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: na
---

---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: na
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: na
---

---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: na
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: na
---

---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: na
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: na
---
---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: passed
  - task: run_c_examples
    status: na
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: passed
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: passed
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: passed
---
"dependencies": [
"@stdlib/math/base/napi/binary",
"@stdlib/math/base/assert/is-nan",
"@stdlib/math/base/special/exp"
Copy link
Contributor

Choose a reason for hiding this comment

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

remove is-nan and exp and add the header files for ln and NINF

"@stdlib/math/base/special/exp",
"@stdlib/math/base/special/asin",
"@stdlib/math/base/special/sqrt",
"@stdlib/constants/float64/pi"
Copy link
Contributor

Choose a reason for hiding this comment

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

you'll have to remove all of these as it has not been used anywhere instead add the files suggested above.

#include "stdlib/stats/base/dists/lognormal/logcdf.h"
#include "stdlib/math/base/special/ln.h"
#include "stdlib/constants/float64/ninf.h"

Copy link
Contributor

@Neerajpathak07 Neerajpathak07 Jan 13, 2025

Choose a reason for hiding this comment

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

need to add #include "stdlib/stats/base/dists/normal/logcdf.h" in order to reference stdlib_base_dists_normal_logcdf
But stdlib/stats/base/dists/normal/logcdf does not have C implementation so this PR will have to wait till we have that.
You can go ahead and draft this PR for the time being

*/
double stdlib_base_dists_lognormal_logcdf( const double x, const double mu, const double sigma ) {
double lx = ( x <= 0.0 ) ? STDLIB_CONSTANT_FLOAT64_NINF : stdlib_base_ln( x );
return stdlib_base_nlogcdf( lx, mu, sigma );
Copy link
Contributor

@Neerajpathak07 Neerajpathak07 Jan 13, 2025

Choose a reason for hiding this comment

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

This reference is incorrect it should be stdlib_base_dists_normal_logcdf as it comes from stats/base/dists/normal/logcdf

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but we implement for stats/base/dists/lognormal/logcdf

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm sorry I didn't understand your point

Copy link
Contributor Author

@saurabhraghuvanshii saurabhraghuvanshii Jan 13, 2025

Choose a reason for hiding this comment

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

sorry, I take it wrong, for now I am leaving this pr . in future see

Copy link
Contributor

Choose a reason for hiding this comment

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

Though I would suggest you to draft this PR for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

today I try once more and change in manifest from binary to ternary so old error no showing new error are code: 'MODULE_NOT_FOUND', requireStack: [ '/home/saurabh/Gsoc/stdlib-gs/lib/node_modules/@stdlib/utils/library-manifest/lib/main.js', '/home/saurabh/Gsoc/stdlib-gs/lib/node_modules/@stdlib/utils/library-manifest/lib/index.js', '/home/saurabh/Gsoc/stdlib-gs/lib/node_modules/@stdlib/stats/base/dists/lognormal/logcdf/[eval]' ] }

@saurabhraghuvanshii saurabhraghuvanshii marked this pull request as draft January 13, 2025 12:46
@stdlib-bot stdlib-bot removed the Needs Review A pull request which needs code review. label Jan 13, 2025
@@ -38,12 +38,9 @@
"libraries": [],
"libpath": [],
"dependencies": [
"@stdlib/math/base/napi/unary",
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no need to make these changes in the manifest.json of math/base/special/ln,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is nothing show like this in manifest.json of math/base/special/ln in repo

Copy link
Contributor

Choose a reason for hiding this comment

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

What I meant was that there was no need to change the block of code in math/base/special/ln so it will be good to revert it back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is depends on base/dists/normal/logcdf , which is not implement at that moment.

"libpath": [],
"dependencies": [
"@stdlib/math/base/napi/binary",
"@stdlib/math/base/assert/is-nan",
Copy link
Contributor

@Neerajpathak07 Neerajpathak07 Jan 15, 2025

Choose a reason for hiding this comment

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

Suggested change
"@stdlib/math/base/assert/is-nan",
"@stdlib/math/base/special/ln",

This is what I meant by adding the header file for manifest.json

"dependencies": [
"@stdlib/math/base/napi/binary",
"@stdlib/math/base/assert/is-nan",
"@stdlib/math/base/special/exp"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"@stdlib/math/base/special/exp"
"@stdlib/constants/float64/ninf"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Statistics Issue or pull request related to statistical functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC]: Add C implementation for @stdlib/stats/base/dists/lognormal/logcdf
3 participants