Skip to content

Test fixes to sparse_center_data and center_data#3

Closed
MechCoder wants to merge 3 commits intojnothman:fix_centeringfrom
MechCoder:fixes
Closed

Test fixes to sparse_center_data and center_data#3
MechCoder wants to merge 3 commits intojnothman:fix_centeringfrom
MechCoder:fixes

Conversation

@MechCoder
Copy link
Copy Markdown

No description provided.

Had been normalizing to unit/n_samples variance since 295dc3c
Not that I understand why this is a feature, but it's tested.
@MechCoder MechCoder changed the title WIP: Test fixes to sparse_center_data and center_data Test fixes to sparse_center_data and center_data and some optimisation in sparsefuncs Mar 26, 2014
@MechCoder
Copy link
Copy Markdown
Author

Never mind the prev comment, I split it across another PR

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

When I said there may be stability benefits to not dividing, that wasn't because we were creating too small a number to represent. Unless you have numbers with large negative exponents, dividing by a power of two is lossless, as far as I understand (but am far from an expert). Dividing, or multiplying, by some other number may be lossy, so this change is not beneficial (except that by keeping the old functionality the tests don't need fixing)

@jnothman
Copy link
Copy Markdown
Owner

You've not pushed the sparsefuncs changes.

@jnothman
Copy link
Copy Markdown
Owner

But I'm not sure if they quite belong in the same PR anyway...?

@jnothman
Copy link
Copy Markdown
Owner

I think ym statement about the current version being good for stability is
probably nonsense anyway....

On 27 March 2014 00:36, Manoj Kumar notifications@github.com wrote:

Some more refactoring to be done in sparsfuncs.pyx

You can merge this Pull Request by running

git pull https://github.com/Manoj-Kumar-S/scikit-learn fixes

Or view, comment on, or merge it at:

#3
Commit Summary

  • FIX: Test fixes to sparse_center_data and center_data

File Changes

Patch Links:

Reply to this email directly or view it on GitHubhttps://github.com//pull/3
.

@MechCoder
Copy link
Copy Markdown
Author

@jnothman I removed the comment about numerical stability. for now this can be merged.

@MechCoder MechCoder changed the title Test fixes to sparse_center_data and center_data and some optimisation in sparsefuncs Test fixes to sparse_center_data and center_data Mar 27, 2014
@jnothman
Copy link
Copy Markdown
Owner

Assuming unit variance isn't something we want..?

On 27 March 2014 14:53, Manoj Kumar notifications@github.com wrote:

@jnothman https://github.com/jnothman I removed the comment about
numerical stability. for now this can be merged.

Reply to this email directly or view it on GitHubhttps://github.com//pull/3#issuecomment-38766830
.

@MechCoder
Copy link
Copy Markdown
Author

Yes, as I said before, I don't still completely understand why this is there. But I suppose it is better to keep it as it is, till someone has a good explanation.

@MechCoder MechCoder closed this Mar 30, 2014
@MechCoder MechCoder deleted the fixes branch March 30, 2014 11:23
jnothman pushed a commit that referenced this pull request Dec 11, 2016
…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
jnothman pushed a commit that referenced this pull request Jun 27, 2017
* add test for _preprocess_data and make it consistent

* fix pep8

* add doc, cast systematically y in X.dtype and update test_coordinate_descent.py

* test if input values don't change with copy=True

* test if input values don't change with copy=True #2

* fix doc

* fix doc #2

* fix doc #3
jnothman pushed a commit that referenced this pull request Oct 4, 2018
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.

2 participants