Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[MRG+1] Reenable MKL for python 3.5 in Travis #6661

Merged
merged 2 commits into from May 3, 2016
Merged

[MRG+1] Reenable MKL for python 3.5 in Travis #6661

merged 2 commits into from May 3, 2016

Conversation

lesteve
Copy link
Member

@lesteve lesteve commented Apr 14, 2016

Trying to fix #6279.

@ogrisel
Copy link
Member

ogrisel commented Apr 15, 2016

LGTM, thanks @lesteve !

@ogrisel ogrisel changed the title [WIP] Reenable MKL for python 3.5 in Travis [MRG+1] Reenable MKL for python 3.5 in Travis Apr 15, 2016
@ogrisel
Copy link
Member

ogrisel commented Apr 18, 2016

@duchesnay do you have an opinion on the eps trick introduced by @lesteve to stabilize the PLS test to work with MKL?

@lesteve
Copy link
Member Author

lesteve commented Apr 19, 2016

Just some additional background: the iterative procedure in _nipals_twoblocks_inner_loop has a pathological fixed point when the first column of Y has only zeros. In this case x_weights is going to be an array of zeros. Adding a small epsilon allows to move away from this pathological fixed point and converge to a more reasonable solution for x_weights.

@lesteve
Copy link
Member Author

lesteve commented Apr 28, 2016

Anyone else, maybe @amueller @rvraghav93 @TomDLT or @arthurmensch?

@TomDLT
Copy link
Member

TomDLT commented Apr 28, 2016

I can reproduce on the bug on:

Linux-3.16.0-4-amd64-x86_64-with-debian-8.4
('Python', '2.7.11 |Anaconda 2.5.0 (64-bit)| \n[GCC 4.4.7 20120313 (Red Hat 4.4.7-1)]')
('NumPy', '1.10.4')
('SciPy', '0.16.1')

The fix works correctly and seems reasonable.
+1

@arthurmensch
Copy link
Contributor

Looks fine, maybe add a comment line so that we keep track of bug fixes in this module ?

@raghavrv
Copy link
Member

LGTM too :)

@lesteve lesteve force-pushed the fix-test-pls-scale-and-stability branch from 63ae9f8 to bc5b670 Compare May 3, 2016 14:04
@lesteve
Copy link
Member Author

lesteve commented May 3, 2016

Looks fine, maybe add a comment line so that we keep track of bug fixes in this module ?

Added a comment to explain the trick. Anything more needed?

@TomDLT TomDLT merged commit 22cf46e into scikit-learn:master May 3, 2016
@TomDLT
Copy link
Member

TomDLT commented May 3, 2016

Thanks @lesteve !

@lesteve lesteve deleted the fix-test-pls-scale-and-stability branch May 3, 2016 14:30
@lesteve lesteve mentioned this pull request May 13, 2016
Merged
olologin pushed a commit to olologin/scikit-learn that referenced this pull request Aug 24, 2016
@lesteve @olologin
[MRG+1] Reenable MKL for python 3.5 in Travis (scikit-learn#6661)
59e5715 
* Enable MKL again

* Fix sklearn.cross_decomposition.test_pls.test_scale_and_stability
TomDLT pushed a commit to TomDLT/scikit-learn that referenced this pull request Oct 3, 2016
@lesteve @TomDLT
[MRG+1] Reenable MKL for python 3.5 in Travis (scikit-learn#6661)
1f9970b 
* Enable MKL again

* Fix sklearn.cross_decomposition.test_pls.test_scale_and_stability
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Waiting for Reviewer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

test_pls.test_scale_and_stability failure
5 participants
@lesteve @ogrisel @TomDLT @arthurmensch @raghavrv