-
Notifications
You must be signed in to change notification settings - Fork 86
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
Lmm/add seeds in more algorithms #164
Conversation
Codecov Report
@@ 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
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
37cca2a
to
89d06e1
Compare
89d06e1
to
d5dbdbe
Compare
pub fn train_test_split<T: RealNumber, M: Matrix<T>>( | ||
x: &M, | ||
y: &M::RowVector, | ||
test_size: f32, | ||
shuffle: bool, | ||
seed: Option<u64>, |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
* 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>
* 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>
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 ofrand
crate so it can be used in WASM targets that don't havegetrandom
support.Fixes #162
Adds a new
js
feature that can be enabled in wasm targets for explicit browser support.