smartcorelib / smartcore

A comprehensive library for machine learning and numerical computing. The library provides a set of tools for linear algebra, numerical computing, optimization, and enables a generic, powerful yet still efficient approach to machine learning.
https://smartcorelib.org/
Apache License 2.0
671 stars 76 forks source link

Remove some allocations #262

Closed rubdos closed 1 year ago

rubdos commented 1 year ago

Performance improvements, i.e. #260 / #261

Fixes no specific issue

Checklist

Current behaviour

smartcore svm is a bit slow

New expected behaviour

smartcore svm is a bit less slow! About 4% faster training in my very simple 5 features x 159 samples test.

Change logs

This removes some allocations by reusing vectors and clearing them in iterations.

codecov-commenter commented 1 year ago

Codecov Report

Merging #262 (59c5993) into development (9cd7348) will increase coverage by 0.02%. The diff coverage is 41.93%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@               Coverage Diff               @@
##           development     #262      +/-   ##
===============================================
+ Coverage        45.38%   45.40%   +0.02%     
===============================================
  Files               85       85              
  Lines             7231     7232       +1     
===============================================
+ Hits              3282     3284       +2     
+ Misses            3949     3948       -1     
Impacted Files Coverage Δ
src/linalg/basic/vector.rs 50.00% <0.00%> (-5.56%) :arrow_down:
src/svm/svc.rs 35.66% <47.82%> (+0.88%) :arrow_up:
src/svm/svr.rs 46.03% <57.14%> (+0.39%) :arrow_up:

... and 13 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

rubdos commented 1 year ago

Second commit makes training another 14% faster on my test data.

Mec-iS commented 1 year ago

thanks a lot, i will look into it as soon as possible. Please run rustfmt src/*.rs to avoid the linting test failing.

rubdos commented 1 year ago

FWIW, rustfmt does not complain about my changes; those lines are unrelated to this PR. I'll gladly run rustfmt if you still want me to, however.

I currently notice that my accuracy estimation (possibly train_test_split) is the slowest part in my test code. I'm playing with KNN now, however (Model trained in 1921 ms; accuracy 0.9921170101624086 (estimated in 189165 ms)).

The difference between training and estimation is only train_test_split and ClassificationMetricsOrd::accuracy().get_score...

Mec-iS commented 1 year ago

good job!

Tomorrow I will have some time at the computer to take a closer look to your contribution.

clippy is complaining for unnecessary parenthesis. For the sake of making the tests pass please apply the requested minor changes even if present in other files.

Thanks again for your time spent using Smartcore.

morenol commented 1 year ago

I can create a PR fixing clippy in a couple of hours. That is probably related to the new lints added to the stable version released a couple of days ago

Mec-iS commented 1 year ago

thanks Luis 👍 obviously we want 100% to stick with clippy standards.

morenol commented 1 year ago

PR opened at: https://github.com/smartcorelib/smartcore/pull/263

We can merge that first. Then we can rebase these changes. Hopefully, CI will be green after that

morenol commented 1 year ago

@rubdos could you rebase your changes? It seems that there are not conflicts

rubdos commented 1 year ago

If you rebase your branch with current development I will merge.

Done!