Skip to content

Least squares cholesky#164

Merged
bvenn merged 7 commits intofslaborg:developerfrom
Recktenwald:LeastSquaresCholesky
Jan 31, 2022
Merged

Least squares cholesky#164
bvenn merged 7 commits intofslaborg:developerfrom
Recktenwald:LeastSquaresCholesky

Conversation

@Recktenwald
Copy link
Copy Markdown

Thank you for contributing to FSharp.Stats. Please take the time to tell us a bit more about your PR.

This is related to Issue #162

Closes #162

Please list the changes introduced in this PR

  • Add the option to use Cholesky Decomposition for OLS.
  • Add tests for the univariable and multivariable version.

@Recktenwald
Copy link
Copy Markdown
Author

If I'm reading the failed checks correctly, I got 'unlucky' with the random numbers for the test vector / test matrix.
It does not seem like a problem in the code itself, just that Cholesky is relatively unstable.
I will hard code a testcase instead.

@Recktenwald
Copy link
Copy Markdown
Author

Oh I just noticed that I was giving the univariable testcase the same y as the multivariable case, which has a bigger error term. I fixed that.
Would it still be better style to have a hardcoded testcase, than a random one?

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #164 (acbdb42) into developer (cc228f4) will increase coverage by 0.41%.
The diff coverage is 77.41%.

Impacted file tree graph

@@              Coverage Diff              @@
##           developer     #164      +/-   ##
=============================================
+ Coverage      17.37%   17.79%   +0.41%     
=============================================
  Files            114      114              
  Lines           9281     9312      +31     
  Branches        1148     1150       +2     
=============================================
+ Hits            1613     1657      +44     
+ Misses          7390     7369      -21     
- Partials         278      286       +8     
Impacted Files Coverage Δ
src/FSharp.Stats/Algebra/LinearAlgebra.fs 11.66% <0.00%> (-0.20%) ⬇️
tests/FSharp.Stats.Tests/Main.fs 0.00% <0.00%> (ø)
src/FSharp.Stats/Fitting/LinearRegression.fs 3.33% <50.00%> (+3.33%) ⬆️
tests/FSharp.Stats.Tests/Fitting.fs 96.00% <94.11%> (-4.00%) ⬇️
...Sharp.Stats/Algebra/LinearAlgebraServiceManaged.fs 17.91% <100.00%> (+3.72%) ⬆️
src/FSharp.Stats/Distributions/Continuous.fs 14.88% <0.00%> (+2.87%) ⬆️
src/FSharp.Stats/Random.fs 25.00% <0.00%> (+6.25%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc228f4...acbdb42. Read the comment docs.

@bvenn
Copy link
Copy Markdown
Member

bvenn commented Nov 4, 2021

If the test with random samples may fail, a discrete case is preferred. Otherwise a hypothetical failure in the future would lead to unnecessary confusion while the methods works perfectly fine. If a failure of this method should be impossible the test you provided seems fine.

@bvenn bvenn merged commit 85bff9e into fslaborg:developer Jan 31, 2022
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.

Add an additional OLS implementation based on Cholesky Decomposition[Feature Request]

3 participants