feat: add JS implementation of math/base/special/legendre-polynomial#10932
feat: add JS implementation of math/base/special/legendre-polynomial#10932officiallyanee wants to merge 5 commits intostdlib-js:developfrom
math/base/special/legendre-polynomial#10932Conversation
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: passed
- task: lint_package_json
status: passed
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: passed
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: passed
- task: lint_javascript_tests
status: passed
- task: lint_javascript_benchmarks
status: passed
- task: lint_python
status: missing_dependencies
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: passed
- task: lint_license_headers
status: passed
---
|
/stdlib update-copyright-years |
Coverage Report
The above coverage report was generated for the changes in this PR. |
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: na
- task: lint_package_json
status: na
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: na
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: missing_dependencies
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
…a test
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: na
- task: lint_package_json
status: na
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: passed
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: passed
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: missing_dependencies
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
Neerajpathak07
left a comment
There was a problem hiding this comment.
@officiallyanee Thanks for this!! A few comments:-
Why are we missing the repl.txt & LICENSE files?
| for ( i = 0; i < 100; i++ ) { | ||
| n = uniform( -50.0, 50.0 ); | ||
| x = uniform( -1.0, 1.0 ); | ||
| v = legendrepoly( n, x ); | ||
| console.log( 'P_%d(%d) = %d', n, x, v ); | ||
| } |
There was a problem hiding this comment.
Apparently we have moved away from using a for to a recursive method using:-
var logEachMap = require( '@stdlib/console/log-each-map' );| expected = integerN.expected; | ||
|
|
||
| for ( i = 0; i < x.length; i++ ) { |
There was a problem hiding this comment.
| expected = integerN.expected; | |
| for ( i = 0; i < x.length; i++ ) { | |
| expected = integerN.expected; | |
| for ( i = 0; i < x.length; i++ ) { |
applies here and below!!
| continue; | ||
| } | ||
| delta = abs( v - expected[ i ] ); | ||
| tol = 1e13 * EPS * abs( expected[ i ] ); |
There was a problem hiding this comment.
Increasing the tolerance by such huge value in-order to pass the test cases would not be viable. If the test cases are failing by large diffs then it might be better to take another closer look at the JS implementation and also see if your generated fixtures are well within the precision limit.
There was a problem hiding this comment.
Oh yes, agreed! I looked into the problem with hyp2f1 and had a PR to improve it too: #11325 I will get back to this to see how much they improve meanwhile I will mark it draft for now
| continue; | ||
| } | ||
| delta = abs( v - expected[ i ] ); | ||
| tol = 3700.0 * EPS * abs( expected[ i ] ); |
There was a problem hiding this comment.
same comment here and below as well!!
|
Thanks for all the suggestions @Neerajpathak07 . I will add the needful soon! |
|
Another comment: similar to |
|
/stdlib merge |
|
@officiallyanee You're right. Looking at SciPy, they also have evaluating a Legendre polynomial in their special namespace (ref: https://docs.scipy.org/doc/scipy/reference/generated/scipy.special.eval_legendre.html#scipy.special.eval_legendre). It is fine to leave as is. Thanks for correcting. |
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes. report:
Resolves #121 .
This pull request:
math/base/special/legendre-polynomialRelated Issues
This pull request has the following related issues:
Questions
No.
Other
No.
Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
I could not understand initially why were tests were failing for non-integer value of n using lower tolerance so i did a ai code review and found a possible cause is hyp2f1
@stdlib-js/reviewers