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 #163

Closed Recktenwald closed 2 years ago

Recktenwald commented 2 years ago

This is adding a least squares algorithm based on the Cholesky Decomposition, as shown in Issue 162 https://github.com/fslaborg/FSharp.Stats/issues/162

As a side note: My autoformatter didn't like lines like let LU A = ... and would automatically change it to let LUA = ... so I added some parentheses and type annotations at such places to avoid that, and because I think it is good style to be explicit.

codecov-commenter commented 2 years ago

Codecov Report

Merging #163 (c3dbb4c) into developer (cc228f4) will decrease coverage by 0.04%. The diff coverage is 12.54%.

Impacted file tree graph

@@              Coverage Diff              @@
##           developer     #163      +/-   ##
=============================================
- Coverage      17.37%   17.33%   -0.05%     
=============================================
  Files            114      114              
  Lines           9281     9314      +33     
  Branches        1148     1149       +1     
=============================================
+ Hits            1613     1615       +2     
- Misses          7390     7421      +31     
  Partials         278      278              
Impacted Files Coverage Δ
src/FSharp.Stats/Fitting/LinearRegression.fs 0.00% <0.00%> (ø)
...Sharp.Stats/Algebra/LinearAlgebraServiceManaged.fs 13.62% <15.60%> (-0.57%) :arrow_down:
src/FSharp.Stats/Algebra/LinearAlgebra.fs 13.11% <16.12%> (+1.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...c3dbb4c. Read the comment docs.

nhirschey commented 2 years ago

Nice idea for a contribution. I think you could make it easier for the maintainers to evaluate your changes if you

  1. Removed all your auto-formatting. It adds a lot of noise and makes it hard to separate the real changes from the formatting changes.
  2. Added a test to verify that your implementation is correct.
bvenn commented 2 years ago

This is a nice one @Recktenwald 🚀 As @nhirschey already mentioned if would be great, if you could open a RP without the autoformatted changes. It makes it hard to identify the points of your contribution regarding the Cholesky decomposition. I'm currently looking into fantomas to format the FSharp.Stats codebase.

Additionally could you please provide an description for leastSquaresCholesky and a test.

Thanks for reporting the missing template. I'm going to fix it.

Recktenwald commented 2 years ago

I rebased and removed two of the commits that mainly included formatting. There is still quite a bit noise left but it is more localized. I'm too inexperienced with git to do much about those. At this point, it would probably be more painless to just delete my repo, delete this PR, make a new fork, and paste the actual content in again. Do you think that's necessary? Would that break something?

I have not added a test, yet. I'd wait with that until you give me the green light with respect to the remaining noisy formatting changes.

bvenn commented 2 years ago

Probably that would be the easiest option for you to create a clean branch, add your changes, and file a new PR.