8000 WIP #10598 <enet_path does not pass params to coordinate descent solver>(Updated) by kkang2097 · Pull Request #10622 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

WIP #10598 <enet_path does not pass params to coordinate descent solver>(Updated) #10622

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
Closed
wants to merge 3 commits into from
Closed

WIP #10598 <enet_path does not pass params to coordinate descent solver>(Updated) #10622

wants to merge 3 commits into from

Conversation

kkang2097
Copy link
Contributor
@kkang2097 kkang2097 commented Feb 12, 2018 •
edited
Loading

Updated the code, sorry for putting in lackluster pull requests. Will put in more attention to detail, something silly like putting in commands after the function ended was easy to prevent.

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Resolves #10598

Any other comments?

@kkang2097
Resolves #10598
3b52b86 
Updated the code, sorry for putting in lackluster pull requests. Will put in more attention to detail, something silly like putting in commands after the function ended was easy to prevent.
@agramfort
Copy link
Member

you need to add a test. I would start with the test by trying to pass a non valid keyword.

@jnothman
Copy link
Member

Please put "resolves #xxx" in pr description, not title. Ping once you have test for a review

@kkang2097 kkang2097 changed the title Resolves #10598 <enet_path does not pass params to coordinate descent solver>(Updated) WIP #10598 <enet_path does not pass params to coordinate descent solver>(Updated) Feb 18, 2018
@kkang2097
Copy link
Contributor Author
kkang2097 commented Feb 18, 2018 •
edited
Loading

Just tested the code.
Inputting this...


from sklearn.linear_model import enet_path
from sklearn import datasets

diabetes = datasets.load_diabetes()
X = diabetes.data
y = diabetes.target


enet_path(X,y, precomputed = True)

returns

runfile('C:/Users/Elliot/Desktop/temp_py.py', wdir='C:/Users/Elliot/Desktop')
Traceback (most recent call last):

File "", line 1, in
runfile('C:/Users/Elliot/Desktop/temp_py.py', wdir='C:/Users/Elliot/Desktop')

File "C:\Users\Elliot\Anaconda3\lib\site-packages\spyder\utils\site\sitecustomize.py", line 705, in runfile
execfile(filename, namespace)

File "C:\Users\Elliot\Anaconda3\lib\site-packages\spyder\utils\site\sitecustomize.py", line 102, in execfile
exec(compile(f.read(), filename, 'exec'), namespace)

File "C:/Users/Elliot/Desktop/temp_py.py", line 10, in
enet_path(X,y, precomputed = True)

File "C:\Users\Elliot\Anaconda3\lib\site-packages\sklearn\linear_model\coordinate_descent.py", line 510, in enet_path
raise ValueError("One or more parameters are invalid")

ValueError: One or more parameters are invalid

while adding any valid arguments to the code makes it run smoothly.

That being said, I still don't get why my code doesn't pass all the tests on Github's build tests. It compiles
perfectly fine on my computer/IDE.

@jnothman
Copy link
Member

A test needs to be a piece of code in the corresponding tests directory that successfully runs only if your change is working. Also, this should match other unexpected keyword argument errors, and should be a TypeError rather than a ValueError

@kkang2097
Copy link
Contributor Author
kkang2097 commented Feb 19, 2018 •
edited
Loading

The code works for enet_path (and when I manually put in invalid and valid function arguments), but when running it against test_enet_path, an error pops up saying
"For mono-task outputs, use ElasticNetCV". @jnothman , is my code invalid or does test_enet_path need to be fixed?

@jnothman
Copy link
Member

Could you provide a fully reproducible code snippet and the traceback for that error?

@kkang2097
Copy link
Contributor Author
kkang2097 commented Feb 20, 2018 •
edited
Loading

Operating System: Windows 10
IDE: Spyder (current version)
Version of Python: 3.6
Version of sklearn: 0.19.1

Error: So the error pops up when I replace params.get with params.pop (in the enet_path() method in the coordinate_descent.py file, that is). Those are lines 433, 434, 438, 439 of the coordinate_descent.py file where I replace params.get with params.pop ...

I think it's because the test_enet_path method checks if all the arguments inside of **params for enet_path when the code starts running are also present when enet_path finishes and returns an output.

I dug into the files indicated in the traceback, and the second line below seemed to raise the error:
(this is an excerpt from the traceback)

File "C:\Users\Elliot\Anaconda3\lib\site-packages\sklearn\utils\testing.py", line 291, in wrapper
return fn(*args, **kwargs)

How to reproduce:

  1. Replace params.get with params.pop in coordinate_descent.py for the enet_path method.

  2. Test the new code with test_enet_path()

Code used:

from sklearn.linear_model import enet_path
from sklearn.linear_model.tests.test_coordinate_descent import test_enet_path

test_enet_path()

Traceback:

runfile('C:/Users/Elliot/Desktop/temp_py.py', wdir='C:/Users/Elliot/Desktop')
Traceback (most recent call last):

File "", line 1, in
runfile('C:/Users/Elliot/Desktop/temp_py.py', wdir='C:/Users/Elliot/Desktop')

File "C:\Users\Elliot\Anaconda3\lib\site-packages\spyder\utils\site\sitecustomize.py", line 705, in runfile
execfile(filename, namespace)

File "C:\Users\Elliot\Anaconda3\lib\site-packages\spyder\utils\site\sitecustomize.py", line 102, in execfile
exec(compile(f.read(), filename, 'exec'), namespace)

File "C:/Users/Elliot/Desktop/temp_py.py", line 12, in
test_enet_path()

File "C:\Users\Elliot\Anaconda3\lib\site-packages\sklearn\linear_model\tests\test_coordinate_descent.py", line 246, in test_enet_path
ignore_warnings(clf.fit)(X, y)

File "C:\Users\Elliot\Anaconda3\lib\site-packages\sklearn\utils\testing.py", line 291, in wrapper
return fn(*args, **kwargs)

File "C:\Users\Elliot\Anaconda3\lib\site-packages\sklearn\linear_model\coordinate_descent.py", line 1104, in fit
"%sCV" % (model_str))

ValueError: For mono-task outputs, use ElasticNetCV

Base automatically changed from master to main January 22, 2021 10:50
@hongshaoyang hong 9C54 shaoyang mentioned this pull request Feb 7, 2021
Merged
@lorentzenchr
Copy link
Member

I'll close in favor of #19391.

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
module:linear_model Stalled
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

enet_path does not pass params to coordinate descent solver
5 participants
@kkang2097 @agramfort @jnothman @lorentzenchr @amueller
0