fslaborg / FSharp.Stats

statistical testing, linear algebra, machine learning, fitting and signal processing in F#
https://fslab.org/FSharp.Stats/
Other
205 stars 54 forks source link

Least squares cholesky #164

Closed Recktenwald closed 2 years ago

Recktenwald commented 2 years ago

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 https://github.com/fslaborg/FSharp.Stats/issues/162

Closes https://github.com/fslaborg/FSharp.Stats/issues/162

Please list the changes introduced in this PR

Recktenwald commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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%) :arrow_down:
tests/FSharp.Stats.Tests/Main.fs 0.00% <0.00%> (ø)
src/FSharp.Stats/Fitting/LinearRegression.fs 3.33% <50.00%> (+3.33%) :arrow_up:
tests/FSharp.Stats.Tests/Fitting.fs 96.00% <94.11%> (-4.00%) :arrow_down:
...Sharp.Stats/Algebra/LinearAlgebraServiceManaged.fs 17.91% <100.00%> (+3.72%) :arrow_up:
src/FSharp.Stats/Distributions/Continuous.fs 14.88% <0.00%> (+2.87%) :arrow_up:
src/FSharp.Stats/Random.fs 25.00% <0.00%> (+6.25%) :arrow_up:

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 commented 2 years ago

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.