Skip to content

FIX Bounds in anisotropic GP hyperparameter optimization #2867

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.

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

Closed
wants to merge 2 commits into from

Conversation

jmetzen
Copy link
Member

@jmetzen jmetzen commented Feb 18, 2014

Currently, if multi-dimensional thetaU and thetaL are given as bounds for anisotropic GP hyperparameter optimization, only the bounds in the last dimensions are obeyed (see added unittest). This pull request fixes this bug. The bug was related to https://stackoverflow.com/questions/10452770/python-lambdas-binding-to-local-values

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling abf3bf4 on jmetzen:gp_fix into 4403d33 on scikit-learn:master.

@AlexanderFabisch
Copy link
Member

This is exactly the same solution as in #2798 with another unit test. :)

@jmetzen
Copy link
Member Author

jmetzen commented Feb 19, 2014

That's true. I would recommend to merge #2798 in order to prevent that we get a third bug fix.

@AlexanderFabisch
Copy link
Member

I don't know who is responsible for this part at the moment. But this is a tiny patch. @ogrisel or @larsmans, could you take a look at this?

random_start=random_start, verbose=False)
gp.fit(X, y)
y_pred, MSE = gp.predict(X, eval_MSE=True)

assert_true(np.allclose(y_pred, y) and np.allclose(MSE, 0.))

assert_true(np.all(gp.theta_ >= thetaL)) # Lower bounds of hyperparameters
assert_true(np.all(gp.theta_ <= thetaU)) # Upper bounds of hyperparameters
Copy link
Member

Choose a reason for hiding this comment

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

PEP8:

tests/test_gaussian_process.py:73:45: E261 at least two spaces before inline comment
tests/test_gaussian_process.py:74:45: E261 at least two spaces before inline comment

@larsmans
Copy link
Member

I have never even looked at this code. Linear classifiers and trees are my game :)

@jmetzen
Copy link
Member Author

jmetzen commented Jun 23, 2014

Could someone review this bugfix PR and merge it such that the issue is fixed in the 0.15 release?
Should be a done in a matter of minutes...

@AlexanderFabisch
Copy link
Member

@ogrisel This is something which you might want to have included in 0.15.

larsmans pushed a commit that referenced this pull request Jun 25, 2014
@larsmans
Copy link
Member

Merged as 9789fdb.

@larsmans larsmans closed this Jun 25, 2014
@GaelVaroquaux
Copy link
Member

Merged as 9789fdb.

Thanks for merging!

@jmetzen jmetzen deleted the gp_fix branch September 15, 2014 14:13
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.

5 participants