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

statistical functions #554

Merged
merged 35 commits into from Oct 31, 2020
Merged

statistical functions #554

merged 35 commits into from Oct 31, 2020

Conversation

izulin
Copy link
Collaborator

@izulin izulin commented Oct 23, 2020

Context

29 statistical functions EXPON.DIST, EXPONDIST, FISHER, FISHERINV, GAMMA, GAMMA.DIST, GAMMADIST, GAMMALN, GAMMALN.PRECISE, GAMMA.INV, GAMMAINV, GAUSS, BETA.DIST, BETADIST, BETA.INV, BETAINV, BINOM.DIST, BINOMDIST, BINOM.INV, BESSELI, BESSELJ, BESSELK, BESSELY, CHIDIST, CHIINV, CHISQ.DIST, CHISQ.DIST.RT, CHISQ.INV, CHISQ.INV.RT.

How has this been tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature or improvement (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Additional language file or change to the existing one (translations)

Related issue(s):

  1. Add Compatibility functions #152
  2. Add Engineering functions #154
  3. Add Statistical functions #160

Checklist:

  • My code follows the code style of this project,
  • My change requires a change to the documentation,
  • I described the modification in the CHANGELOG.md file.

@izulin izulin marked this pull request as draft October 23, 2020 22:31
@codecov
Copy link

codecov bot commented Oct 23, 2020

Codecov Report

Merging #554 into develop will increase coverage by 0.01%.
The diff coverage is 92.51%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #554      +/-   ##
===========================================
+ Coverage    91.89%   91.91%   +0.01%     
===========================================
  Files          159      161       +2     
  Lines        31666    33491    +1825     
  Branches      2971     3130     +159     
===========================================
+ Hits         29099    30782    +1683     
- Misses        2561     2703     +142     
  Partials         6        6              
Impacted Files Coverage Δ
src/interpreter/plugin/3rdparty/bessel/bessel.ts 84.12% <84.12%> (ø)
src/interpreter/plugin/3rdparty/jstat/jstat.ts 86.54% <86.54%> (ø)
src/i18n/languages/csCZ.ts 100.00% <100.00%> (ø)
src/i18n/languages/daDK.ts 100.00% <100.00%> (ø)
src/i18n/languages/deDE.ts 100.00% <100.00%> (ø)
src/i18n/languages/enGB.ts 100.00% <100.00%> (ø)
src/i18n/languages/esES.ts 100.00% <100.00%> (ø)
src/i18n/languages/fiFI.ts 100.00% <100.00%> (ø)
src/i18n/languages/frFR.ts 100.00% <100.00%> (ø)
src/i18n/languages/huHU.ts 100.00% <100.00%> (ø)
... and 13 more

@izulin izulin marked this pull request as ready for review October 29, 2020 09:37
Copy link
Contributor

@wojciechczerniak wojciechczerniak left a comment

Choose a reason for hiding this comment

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

ksawery2.txt Outdated Show resolved Hide resolved
src/interpreter/plugin/3rdparty/bessel/LICENSE Outdated Show resolved Hide resolved
src/interpreter/plugin/StatisticalPlugin.ts Outdated Show resolved Hide resolved
src/interpreter/plugin/StatisticalPlugin.ts Outdated Show resolved Hide resolved
test/interpreter/function-chisq.inv.rt.spec.ts Outdated Show resolved Hide resolved
test/interpreter/function-gamma.dist.spec.ts Show resolved Hide resolved
test/interpreter/function-gamma.inv.spec.ts Show resolved Hide resolved
test/interpreter/function-binom.inv.spec.ts Outdated Show resolved Hide resolved
test/interpreter/function-binom.inv.spec.ts Outdated Show resolved Hide resolved
@izulin
Copy link
Collaborator Author

izulin commented Oct 29, 2020

Can you add jstat and bassel to deps https://handsontable.github.io/hyperformula/guide/dependencies.html ?

How do I edit this?

Co-authored-by: Wojciech Czerniak <wojciech.czerniak@gmail.com>
@wojciechczerniak
Copy link
Contributor

Copy link
Contributor

@wojciechczerniak wojciechczerniak left a comment

Choose a reason for hiding this comment

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

Functions are good, one more thing is to adjust formatting and linters a little bit.

docs/guide/dependencies.md Outdated Show resolved Hide resolved
src/interpreter/plugin/3rdparty/bessel/bessel.ts Outdated Show resolved Hide resolved
src/interpreter/plugin/3rdparty/bessel/bessel.ts Outdated Show resolved Hide resolved
src/interpreter/plugin/3rdparty/jstat/jstat.ts Outdated Show resolved Hide resolved
izulin and others added 4 commits October 30, 2020 14:57
Co-authored-by: Wojciech Czerniak <wojciech.czerniak@gmail.com>
Co-authored-by: Wojciech Czerniak <wojciech.czerniak@gmail.com>
Copy link
Contributor

@wojciechczerniak wojciechczerniak left a comment

Choose a reason for hiding this comment

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

Thanks! 🥇

Copy link
Collaborator

@bardek8 bardek8 left a comment

Choose a reason for hiding this comment

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

Thanks! It's all good, I only found some small inconsistencies between our results and GS/XL.

['=BETA.DIST(1, 2, 3, 4, 5, 6, 7)'],
])

expect(engine.getCellValue(adr('A1'))).toEqualError(detailedError(ErrorType.NA, ErrorMessage.WrongArgNumber))
Copy link
Collaborator

Choose a reason for hiding this comment

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

GS sets the default value of cumulative to TRUE and returns 1 here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is some weird artefact...

['=BETA.DIST(0.6, 1, 1, FALSE(), 0.6, 0.7)'],
['=BETA.DIST(0.7, 1, 1, FALSE(), 0.6, 0.7)'],
])

Copy link
Collaborator

Choose a reason for hiding this comment

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

Next inconsistency: GS returns 0 for 1st, 4th and 5th test.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

those are also some internally inconsistent values for GS :)

])

expect(engine.getCellValue(adr('A1'))).toEqual(0)
expect(engine.getCellValue(adr('A2'))).toEqualError(detailedError(ErrorType.NUM, ErrorMessage.ValueSmall))
Copy link
Collaborator

Choose a reason for hiding this comment

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

GS returns 0 here

expect(engine.getCellValue(adr('A2'))).toEqualError(detailedError(ErrorType.NUM, ErrorMessage.ValueSmall))
//both products #1 and #2 return NUM for '=BINOM.INV(10, 0, 0.5)', which is incorrect
expect(engine.getCellValue(adr('A3'))).toEqual(0)
//both products #1 and #2 return NUM for '=BINOM.INV(10, 10, 0.5)', which is incorrect
Copy link
Collaborator

Choose a reason for hiding this comment

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

the test is (10,1,0.5)

expect(engine.getCellValue(adr('A1'))).toEqual(1)
expect(engine.getCellValue(adr('A2'))).toBeCloseTo(1.77245385588014, 6)
expect(engine.getCellValue(adr('A3')) as number / 1133278.39212948).toBeCloseTo(1, 6)
expect(engine.getCellValue(adr('A4'))).toBeCloseTo(-0.94530871782981, 6)
Copy link
Collaborator

Choose a reason for hiding this comment

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

GS returns NUM

expect(engine.getCellValue(adr('A1'))).toEqualError(detailedError(ErrorType.NUM, ErrorMessage.ValueSmall))
expect(engine.getCellValue(adr('A2'))).toEqualError(detailedError(ErrorType.NUM, ErrorMessage.ValueSmall))
expect(engine.getCellValue(adr('A3'))).toEqualError(detailedError(ErrorType.NUM, ErrorMessage.ValueSmall))
expect(engine.getCellValue(adr('A4'))).toEqual(1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

XL returns NUM here


expect(engine.getCellValue(adr('A1'))).toEqualError(detailedError(ErrorType.NUM, ErrorMessage.ValueSmall))
expect(engine.getCellValue(adr('A2'))).toEqualError(detailedError(ErrorType.NUM, ErrorMessage.ValueSmall))
expect(engine.getCellValue(adr('A3'))).toEqualError(detailedError(ErrorType.NUM, ErrorMessage.WrongOrder))
Copy link
Collaborator

Choose a reason for hiding this comment

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

XL returns 1 here

@bardek8
Copy link
Collaborator

bardek8 commented Oct 31, 2020

Also please add to the docs/guide/known-limitations.md file a new section, called e.g. Nuances of the implemented functions with all the nuances that you found recently.

@izulin izulin requested a review from bardek8 October 31, 2020 19:13
@izulin izulin merged commit 974c492 into develop Oct 31, 2020
@izulin izulin deleted the pu/stat branch October 31, 2020 21:05
This was referenced Nov 25, 2020
@wojciechczerniak wojciechczerniak mentioned this pull request Dec 9, 2020
38 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants