Skip to content

Lmm/add seeds in more algorithms #164

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

Merged

Conversation

morenol
Copy link
Collaborator

@morenol morenol commented Sep 20, 2022

Adds seeds parameter to multiple algorithms (including SVC) in order to fix the flaky test by using a fixed seed for that test.

Fixes #158

Also, adding the capability to disable std feature of rand crate so it can be used in WASM targets that don't have getrandom support.

Fixes #162

Adds a new js feature that can be enabled in wasm targets for explicit browser support.

@codecov-commenter
Copy link

codecov-commenter commented Sep 20, 2022

Codecov Report

Merging #164 (8d87d47) into development (c21e752) will increase coverage by 0.05%.
The diff coverage is 92.00%.

@@               Coverage Diff               @@
##           development     #164      +/-   ##
===============================================
+ Coverage        84.28%   84.34%   +0.05%     
===============================================
  Files               85       86       +1     
  Lines             9215     9228      +13     
===============================================
+ Hits              7767     7783      +16     
+ Misses            1448     1445       -3     
Impacted Files Coverage Δ
src/model_selection/kfold.rs 88.99% <40.00%> (-2.44%) ⬇️
src/math/num.rs 80.76% <50.00%> (-1.93%) ⬇️
src/cluster/kmeans.rs 85.82% <100.00%> (ø)
src/ensemble/random_forest_classifier.rs 71.72% <100.00%> (+0.68%) ⬆️
src/ensemble/random_forest_regressor.rs 71.42% <100.00%> (+0.84%) ⬆️
src/model_selection/mod.rs 94.04% <100.00%> (+0.07%) ⬆️
src/rand.rs 100.00% <100.00%> (ø)
src/svm/svc.rs 89.73% <100.00%> (-0.03%) ⬇️
src/tree/decision_tree_classifier.rs 83.51% <100.00%> (+0.90%) ⬆️
src/tree/decision_tree_regressor.rs 85.85% <100.00%> (+1.31%) ⬆️
... and 5 more

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

@morenol morenol force-pushed the lmm/add-seeds-in-more-algorithms branch from 37cca2a to 89d06e1 Compare September 20, 2022 23:53
@morenol morenol force-pushed the lmm/add-seeds-in-more-algorithms branch from 89d06e1 to d5dbdbe Compare September 20, 2022 23:58
pub fn train_test_split<T: RealNumber, M: Matrix<T>>(
x: &M,
y: &M::RowVector,
test_size: f32,
shuffle: bool,
seed: Option<u64>,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Backward incompatible change that will require bump of version to 0.3

Copy link
Collaborator

Choose a reason for hiding this comment

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

To avoid breaking the API, an alternative would be to have a static rng, that can be seeded with an api call, and we could make that call at the start of tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think that we are going to do breaking changes in #108, so in my opinion we should not be worried about adding backward incompatible changes like this one as long as we document it in a changelog and we bump the version appropriately

@Mec-iS Mec-iS merged commit 3a44161 into smartcorelib:development Sep 21, 2022
morenol added a commit that referenced this pull request Sep 22, 2022
* Complete grid search params (#166)

* grid search draft

* hyperparam search for linear estimators

* grid search for ensembles

* support grid search for more algos

* grid search for unsupervised algos

* minor cleanup

* Lmm/add seeds in more algorithms (#164)

* Provide better output in flaky tests

* feat: add seed parameter to multiple algorithms

* Update changelog

Co-authored-by: Luis Moreno <morenol@users.noreply.github.com>

* add seed param to search params (#168)

* make default params available to serde (#167)

* add seed param to search params

* make default params available to serde

* lints

* create defaults for enums

* lint

Co-authored-by: morenol <22335041+morenol@users.noreply.github.com>
Co-authored-by: Luis Moreno <morenol@users.noreply.github.com>
morenol added a commit that referenced this pull request Nov 8, 2022
* Provide better output in flaky tests

* feat: add seed parameter to multiple algorithms

* Update changelog

Co-authored-by: Luis Moreno <morenol@users.noreply.github.com>
@morenol morenol deleted the lmm/add-seeds-in-more-algorithms branch January 13, 2023 17:50
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.

Make compatible with non-browser wasm targets Instability in svm::svc::tests::svc_fit_predict -test
4 participants