Skip to content

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

Merged
TomDLT merged 2 commits intoscikit-learn:masterfrom
lesteve:fix-test-pls-scale-and-stability
May 3, 2016
Merged

[MRG+1] Reenable MKL for python 3.5 in Travis#6661
TomDLT merged 2 commits intoscikit-learn:masterfrom
lesteve:fix-test-pls-scale-and-stability

Conversation

@lesteve
Copy link
Copy Markdown
Member

@lesteve lesteve commented Apr 14, 2016

Trying to fix #6279.

@ogrisel
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member Author

lesteve commented Apr 28, 2016

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

@TomDLT
Copy link
Copy Markdown
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
Copy Markdown
Contributor

arthurmensch commented Apr 29, 2016

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

@raghavrv
Copy link
Copy Markdown
Member

raghavrv commented Apr 29, 2016

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
Copy Markdown
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
Copy Markdown
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