[MRG+2] Adding return_std options for models in linear_model/bayes.py by sergeyf · Pull Request #7838 · scikit-learn/scikit-learn

sergeyf · 2016-11-07T20:14:08Z

Reference Issue

The reason for this pull request appears in a conversation for #4844

What does this implement/fix? Explain your changes.

This is the first of two pull requests. The ultimate goal is to add the MICE imputation algorithm to scikit-learn. To do so, we need sklearn's Bayesian regression algorithms to be able to return standard deviations as well as predictions.

This pull requests adds the option return_std to the predict methods of both BayesianRidge and ARDRegression.

Any other comments?

Once this is accepted, I will make a pull request that implements MICE using BayesianRidge by default (which seems more robust to small sample sizes than ARD in my limited experience).

jnothman · 2016-11-07T23:19:52Z

sklearn/linear_model/bayes.py

+        X : {array-like, sparse matrix}, shape = (n_samples, n_features)
+            Samples.
+
+        predict_std : boolean, optional


Please use return_std like GaussianProcessRegressor, and check its docstring for consistency.

Oh my bad - I'll change that to return_std.

sergeyf · 2016-11-07T23:34:59Z

@jnothman I changed all instances of predict_std to return_std. Thanks for pointing that one out.

jnothman · 2016-11-07T23:45:17Z

Previous example renderings:

I wonder if there's a way to visually emphasise the difference in uncertainties between the centre and edges of these plots.

jnothman · 2016-11-07T23:46:37Z

Could you put the new figure in the fourth quadrant of the existing plots?

sergeyf · 2016-11-07T23:52:12Z

The problem is that it the uncertainty grows linearly. I tried zooming out further, but the plot looks qualitatively the same.

One idea is to have a non-linear function f(x), and then use a polynomial kernel to estimate it. I'll give this a shot.

And yes, I'll put the figures in the fourth quadrant.

Thanks.

sergeyf · 2016-11-08T00:34:30Z

@jnothman I tried the polynomial regression thing. Check it out. It works better with BayesianRidge than with ARD, but the idea is there in both I think.

jnothman · 2016-11-08T01:14:31Z

ARD:

BRR:

Inserted with:

ARD:
![ARD](https://$CIRCLE_BUILDNO-843222-gh.circle-artifacts.com/0/home/ubuntu/scikit-learn/doc/_build/html/stable/_images/sphx_glr_plot_ard_004.png)
BRR:   
![BRR](https://$CIRCLE_BUILDNO-843222-gh.circle-artifacts.com/0/home/ubuntu/scikit-learn/doc/_build/html/stable/_images/sphx_glr_plot_bayesian_ridge_004.png)

jnothman · 2016-11-08T01:14:55Z

examples/linear_model/plot_ard.py

+y_mean, y_std = clf_poly.predict(np.vander(X_plot, degree), return_std=True)
+plt.figure(figsize=(6, 5))
+plt.errorbar(X_plot, y_mean, y_std, color='navy',
+        label="Polynomial Bayesian Ridge Regression", linewidth=2)


Incorrect label for ARD

jnothman · 2016-11-20T22:24:04Z

I agree those test failures don't appear to be your problem.

lesteve · 2016-11-21T14:54:03Z

@sergeyf you still have flake8 errors in Travis. My advice is to take some time to have on-the-fly flake8 linting in your editor of choice, it will save you a lot of time in the long run.

sergeyf · 2016-11-21T16:37:15Z

@lesteve Thanks for the suggestion. I use Spyder - I think it has pyflakes integration. I'll dig around. The last two should be fixed now in any case.

amueller

Looks ok, haven't checked the math (yet?)

amueller · 2016-11-23T21:50:24Z

sklearn/linear_model/bayes.py

        X, y = check_X_y(X, y, dtype=np.float64, y_numeric=True)
        X, y, X_offset, y_offset, X_scale = self._preprocess_data(
            X, y, self.fit_intercept, self.normalize, self.copy_X)
+        self.X_offset = X_offset


Should be X_offset_ and X_scale_

amueller · 2016-11-23T21:50:47Z

sklearn/linear_model/bayes.py

        self.lambda_ = lambda_
        self.coef_ = coef_
+        sigma_ = np.dot(Vh.T,
+                        Vh / (eigen_vals_ + lambda_ / alpha_)[:, None])


I prefer np.newaxis over None

amueller · 2016-11-23T21:51:51Z

sklearn/linear_model/bayes.py

        return self

+    def predict(self, X, return_std=False):
+        """Predict using the linear model. In addition to the mean of the


This is not pep 257: https://www.python.org/dev/peps/pep-0257/

Please add an empty line after the first sentence.

amueller · 2016-11-23T21:52:39Z

sklearn/linear_model/bayes.py

        return self
+
+    def predict(self, X, return_std=False):
+        """Predict using the linear model. In addition to the mean of the


Same comment about docstring pep.

amueller · 2016-11-23T21:53:23Z

sklearn/linear_model/tests/test_bayes.py

+        return np.dot(X, w) + b
+
+    def f_noise(X, noise_mult):
+        return f(X) + np.random.randn(X.shape[0])*noise_mult


is that pep8 without space around *? hm

Looks ok, check out: https://www.python.org/dev/peps/pep-0008/#id28

amueller · 2016-11-23T21:54:11Z

sklearn/linear_model/tests/test_bayes.py

+    n_train = 50
+    n_test = 10
+
+    noise_mult = 0.1


does this work if you do a for-loop over multiple noise_mult?

Works fine for Bayesian Ridge, but unfortunately, ARD behaves oddly because it gets rid of a bunch of dimensions. It gets it MOSTLY right. If you set noise_mult = 1.0, ARD will get 1.2 or 1.1, which is good enough for the estimation of standard deviation of noise, but I'd have to bump up decimal=0 for it to still pass the tests for such a large noise. I'll do that for the next commit unless you have another suggestion.

amueller · 2016-11-23T21:55:02Z

sklearn/linear_model/tests/test_bayes.py

    assert_array_almost_equal(clf.predict(test), [1, 3, 4], 2)
+
+
+def test_return_std_bayesian():


If the tests are the same (might have overlooked something?), why not do them both in the same test?

sergeyf · 2016-11-24T04:04:58Z

@amueller I've made the requested fixes. Thanks!

amueller · 2016-11-30T21:26:18Z

can you fix pep8? Integration is failing.

amueller · 2016-11-30T21:27:13Z

Other than that LGTM

sergeyf · 2016-11-30T21:32:13Z

Thanks @amueller, just fixed that last one.

lesteve · 2016-12-01T08:02:16Z

sklearn/linear_model/bayes.py

+
+    R. Salakhutdinov, Lecture notes on Statistical Machine Learning,
+    http://www.utstat.toronto.edu/~rsalakhu/sta4273/notes/Lecture2.pdf#page=15
+    Their beta is our self.beta_


beta_ is not listed in the attributes, did you mean another attribute maybe?

Thanks for the catch. I meant self.alpha_.

lesteve · 2016-12-01T08:04:13Z

sklearn/linear_model/bayes.py

+
+    R. Salakhutdinov, Lecture notes on Statistical Machine Learning,
+    http://www.utstat.toronto.edu/~rsalakhu/sta4273/notes/Lecture2.pdf#page=15
+    Their beta is our self.beta_


Same remark about beta_ not being listed in the attributes.

amueller · 2016-12-01T15:52:13Z

thanks!

…scikit-learn#7838) * initial commit for return_std * initial commit for return_std * adding tests, examples, ARD predict_std * adding tests, examples, ARD predict_std * a smidge more documentation * a smidge more documentation * Missed a few PEP8 issues * Changing predict_std to return_std #1 * Changing predict_std to return_std scikit-learn#2 * Changing predict_std to return_std scikit-learn#3 * Changing predict_std to return_std final * adding better plots via polynomial regression * trying to fix flake error * fix to ARD plotting issue * fixing some flakes * Two blank lines part 1 * Two blank lines part 2 * More newlines! * Even more newlines * adding info to the doc string for the two plot files * Rephrasing "polynomial" for Bayesian Ridge Regression * Updating "polynomia" for ARD * Adding more formal references * Another asked-for improvement to doc string. * Fixing flake8 errors * Cleaning up the tests a smidge. * A few more flakes * requested fixes from Andy * Mini bug fix * Final pep8 fix * pep8 fix round 2 * Fix beta_ to alpha_ in the comments

…scikit-learn#7838) * initial commit for return_std * initial commit for return_std * adding tests, examples, ARD predict_std * adding tests, examples, ARD predict_std * a smidge more documentation * a smidge more documentation * Missed a few PEP8 issues * Changing predict_std to return_std scikit-learn#1 * Changing predict_std to return_std scikit-learn#2 * Changing predict_std to return_std scikit-learn#3 * Changing predict_std to return_std final * adding better plots via polynomial regression * trying to fix flake error * fix to ARD plotting issue * fixing some flakes * Two blank lines part 1 * Two blank lines part 2 * More newlines! * Even more newlines * adding info to the doc string for the two plot files * Rephrasing "polynomial" for Bayesian Ridge Regression * Updating "polynomia" for ARD * Adding more formal references * Another asked-for improvement to doc string. * Fixing flake8 errors * Cleaning up the tests a smidge. * A few more flakes * requested fixes from Andy * Mini bug fix * Final pep8 fix * pep8 fix round 2 * Fix beta_ to alpha_ in the comments

…scikit-learn#7838) * initial commit for return_std * initial commit for return_std * adding tests, examples, ARD predict_std * adding tests, examples, ARD predict_std * a smidge more documentation * a smidge more documentation * Missed a few PEP8 issues * Changing predict_std to return_std #1 * Changing predict_std to return_std #2 * Changing predict_std to return_std #3 * Changing predict_std to return_std final * adding better plots via polynomial regression * trying to fix flake error * fix to ARD plotting issue * fixing some flakes * Two blank lines part 1 * Two blank lines part 2 * More newlines! * Even more newlines * adding info to the doc string for the two plot files * Rephrasing "polynomial" for Bayesian Ridge Regression * Updating "polynomia" for ARD * Adding more formal references * Another asked-for improvement to doc string. * Fixing flake8 errors * Cleaning up the tests a smidge. * A few more flakes * requested fixes from Andy * Mini bug fix * Final pep8 fix * pep8 fix round 2 * Fix beta_ to alpha_ in the comments

…scikit-learn#7838) * initial commit for return_std * initial commit for return_std * adding tests, examples, ARD predict_std * adding tests, examples, ARD predict_std * a smidge more documentation * a smidge more documentation * Missed a few PEP8 issues * Changing predict_std to return_std #1 * Changing predict_std to return_std scikit-learn#2 * Changing predict_std to return_std scikit-learn#3 * Changing predict_std to return_std final * adding better plots via polynomial regression * trying to fix flake error * fix to ARD plotting issue * fixing some flakes * Two blank lines part 1 * Two blank lines part 2 * More newlines! * Even more newlines * adding info to the doc string for the two plot files * Rephrasing "polynomial" for Bayesian Ridge Regression * Updating "polynomia" for ARD * Adding more formal references * Another asked-for improvement to doc string. * Fixing flake8 errors * Cleaning up the tests a smidge. * A few more flakes * requested fixes from Andy * Mini bug fix * Final pep8 fix * pep8 fix round 2 * Fix beta_ to alpha_ in the comments

…scikit-learn#7838) * initial commit for return_std * initial commit for return_std * adding tests, examples, ARD predict_std * adding tests, examples, ARD predict_std * a smidge more documentation * a smidge more documentation * Missed a few PEP8 issues * Changing predict_std to return_std scikit-learn#1 * Changing predict_std to return_std scikit-learn#2 * Changing predict_std to return_std scikit-learn#3 * Changing predict_std to return_std final * adding better plots via polynomial regression * trying to fix flake error * fix to ARD plotting issue * fixing some flakes * Two blank lines part 1 * Two blank lines part 2 * More newlines! * Even more newlines * adding info to the doc string for the two plot files * Rephrasing "polynomial" for Bayesian Ridge Regression * Updating "polynomia" for ARD * Adding more formal references * Another asked-for improvement to doc string. * Fixing flake8 errors * Cleaning up the tests a smidge. * A few more flakes * requested fixes from Andy * Mini bug fix * Final pep8 fix * pep8 fix round 2 * Fix beta_ to alpha_ in the comments

sergeyf added 6 commits November 7, 2016 09:36

initial commit for return_std

c19e2c9

initial commit for return_std

2fad2d5

adding tests, examples, ARD predict_std

a6c0bf3

adding tests, examples, ARD predict_std

f92a860

a smidge more documentation

4bae33d

a smidge more documentation

ea9fad4

sergeyf mentioned this pull request Nov 7, 2016

[MRG] Add KNN strategy for imputation #4844

Closed

raghavrv added this to the 0.19 milestone Nov 7, 2016

raghavrv added the New Feature label Nov 7, 2016

jnothman added the Waiting for Reviewer label Nov 7, 2016

Missed a few PEP8 issues

25c457e

jnothman requested changes Nov 7, 2016

View reviewed changes

sergeyf closed this Nov 7, 2016

sergeyf reopened this Nov 7, 2016

sergeyf added 4 commits November 7, 2016 15:28

Changing predict_std to return_std #1

b905a23

Changing predict_std to return_std scikit-learn#2

0a3ccd2

Changing predict_std to return_std scikit-learn#3

5634ee2

Changing predict_std to return_std final

e817de3

sergeyf changed the title ~~[MRG] Adding predict_std options for models in linear_model/bayes.py~~ [MRG] Adding return_std options for models in linear_model/bayes.py Nov 7, 2016

sergeyf added 2 commits November 7, 2016 16:29

adding better plots via polynomial regression

806818a

trying to fix flake error

2f0bd32

jnothman requested changes Nov 8, 2016

View reviewed changes

sergeyf added 2 commits November 7, 2016 17:29

fix to ARD plotting issue

21ba9d5

fixing some flakes

df3038a

Cleaning up the tests a smidge.

0ded8b7

jnothman mentioned this pull request Nov 20, 2016

[MRG + 2] FIX LogisticRegressionCV to correctly handle string labels #5874

Merged

A few more flakes

1e1392c

amueller reviewed Nov 23, 2016

View reviewed changes

requested fixes from Andy

f7e31f1

Mini bug fix

039ae83

amueller changed the title ~~[MRG+1] Adding return_std options for models in linear_model/bayes.py~~ [MRG+2] Adding return_std options for models in linear_model/bayes.py Nov 30, 2016

Final pep8 fix

092a569

pep8 fix round 2

561ef01

lesteve reviewed Dec 1, 2016

View reviewed changes

Fix beta_ to alpha_ in the comments

5bb4080

amueller merged commit 67a85b8 into scikit-learn:master Dec 1, 2016

sergeyf deleted the MICE branch December 2, 2016 18:00

sergeyf mentioned this pull request Feb 27, 2017

[WIP] Basic version of MICE Imputation #8465

Closed

This was referenced Feb 28, 2017

[WIP] Basic version of MICE Imputation (2nd try) #8476

Closed

[MRG+2] Basic version of MICE Imputation #8478

Merged

		assert_array_almost_equal(clf.predict(test), [1, 3, 4], 2)


		def test_return_std_bayesian():

Uh oh!

Conversation

sergeyf commented Nov 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reference Issue

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sergeyf commented Nov 7, 2016

Uh oh!

jnothman commented Nov 7, 2016

Uh oh!

jnothman commented Nov 7, 2016

Uh oh!

sergeyf commented Nov 7, 2016

Uh oh!

sergeyf commented Nov 8, 2016

Uh oh!

jnothman commented Nov 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnothman commented Nov 20, 2016

Uh oh!

lesteve commented Nov 21, 2016

Uh oh!

sergeyf commented Nov 21, 2016

Uh oh!

amueller left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sergeyf commented Nov 24, 2016

Uh oh!

amueller commented Nov 30, 2016

Uh oh!

amueller commented Nov 30, 2016

Uh oh!

sergeyf commented Nov 30, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amueller commented Dec 1, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sergeyf commented Nov 7, 2016 •

edited

Loading

jnothman commented Nov 8, 2016 •

edited

Loading