chore: improve the implementation of math/base/special/hyp2f1#11325
chore: improve the implementation of math/base/special/hyp2f1#11325officiallyanee wants to merge 3 commits intostdlib-js:developfrom
math/base/special/hyp2f1#11325Conversation
---
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
---
Coverage Report
The above coverage report was generated for the changes in this PR. |
| if ( id > 0.0 ) { | ||
| y *= q; | ||
| } else { | ||
| y1 *= q; | ||
| } |
There was a problem hiding this comment.
This was wrong originally, cephes and scipy both have this but it was not discovered probably because else branch is never hit ( in test cov ) so I changed it.
There was a problem hiding this comment.
To clarify, the bug was in y1 *= q mistakenly being implemented as y = y1 * q, correct?
There was a problem hiding this comment.
Yes, the snippet from cephes for reference:
if( id > 0.0 )
y *= q;
else
y1 *= q;
y += y1;| // FUNCTIONS // | ||
|
|
||
| /** | ||
| * Evaluates 2F1(a, b; b; x) when `b = c` is a negative integer using AMS55 #15.4.2. | ||
| * | ||
| * @private | ||
| * @param {number} a - first parameter | ||
| * @param {number} b - second parameter (equals c, a non-positive integer) | ||
| * @param {number} x - argument | ||
| * @returns {number} function value, or NaN if precision is insufficient | ||
| */ | ||
| function hyp2f1NegCEqualBC( a, b, x ) { | ||
| var collectorMax; | ||
| var collector; | ||
| var sum; | ||
| var k; | ||
|
|
||
| if ( !( abs( b ) < 1.0e5 ) ) { | ||
| return NaN; | ||
| } | ||
| collectorMax = 1.0; | ||
| collector = 1.0; | ||
| sum = 1.0; | ||
| for ( k = 1; k <= -b; k++ ) { | ||
| collector *= ( a + k - 1.0 ) * x / k; | ||
| collectorMax = max( abs( collector ), collectorMax ); | ||
| sum += collector; | ||
| } | ||
| if ( 1.0e-16 * ( 1.0 + ( collectorMax / abs( sum ) ) ) > 1.0e-7 ) { | ||
| return NaN; | ||
| } | ||
| return sum; | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
I will add this in a separate file if that would be better, the implementation for this was taken from scipy.
There was a problem hiding this comment.
Yes, if you don't mind going ahead and moving. Cheers!
| id = round( d ); | ||
|
|
||
| if ( x > 0.9 && !negIntA && !negIntB ) { | ||
| if ( x > 0.85 && !negIntA && !negIntB ) { |
There was a problem hiding this comment.
0.85 was chosen arbitrarily through trial and error, this helped the case I mentioned in the description.
There was a problem hiding this comment.
@officiallyanee Do you happen to know where/why the 0.9 was chosen in the first place?
There was a problem hiding this comment.
Its not explicitly mentioned in the cephes/scipy code but I went through some resources and found this:

ref: https://link.springer.com/article/10.1007/s11075-016-0173-0
I think it also explains why the initial case was performing badly too since |a| and |b| were much larger than |c|
and |x| is also pretty close to 1 so the power series was performing poorly.
In the range 0.85<x<0.9, it was true for every value as long as we keep a, b, c same.
( this is assuming wolfram as a better source of comparision )
|
/stdlib merge |
| for ( i = 0; i < x.length; i++ ) { | ||
| v = hyp2f1( a[ i ], b[ i ], c[ i ], x[ i ] ); | ||
| delta = abs( v - expected[ i ] ); | ||
| tol = 8.0 * EPS * abs( expected[ i ] ); |
There was a problem hiding this comment.
We just had another PR land which migrated hyp2f1 to use ULP-based testing. If you could follow suit here, that would be appreciated!
|
/stdlib merge |
|
@officiallyanee Merged in the latest |
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes. report:
Resolves none.
Description
This pull request:
math/base/special/hyp2f1. It started with this issue:Though the difference still exists but wolfram gives 0.0040689094468919286...
so output now is closer to wolfram.
It also uses some updated implementation from scipy's cephes hyp2f1 with added test cases to test the same.
I will work on C implementation once this gets the green light.
Related 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
{{TODO: add disclosure if applicable}}
@stdlib-js/reviewers