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.

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

"metric" is linear but "metric_bounds" is log #150

Open
arodland opened this issue Jan 13, 2022 · 3 comments
Open

"metric" is linear but "metric_bounds" is log #150

arodland opened this issue Jan 13, 2022 · 3 comments

Comments

@arodland
Copy link

If you do ExpSquaredKernel(metric=30, metric_bounds=[(10, 100)]) it will fail with an unhelpful "non-finite log prior value". Why? Because the initial value is 30, but the bounds are e^10 and e^100, which doesn't include the initial value. Same applies if you do Metric(30, bounds=[(10, 100)], ndim=1). You really wanted to write ExpSquaredKernel(metric=30, metric_bounds=[(np.log(10), np.log(100))]).

It would be nicer to change the interface (either to take the bounds as linear and take the log internally, or rename the arg as log_metric_bounds) but that would be a breaking change, so perhaps you could just clearly document the existing behavior and provide an example?

Also, the code that produces that "non-finite log prior" message knows that it's doing a bounds check, so how about making the message something more like metric prior value 30 outside of bounds [22026.46579, 2.68811714e+43] which would tip people off as to where things went wrong.

@dfm
Copy link
Owner

dfm commented Jan 13, 2022

Yeah - that interface is all a little janky! All of these suggestions are good. I don't have a huge amount of time for george maintenance these days, but I'd be very happy to review a PR.

@arodland
Copy link
Author

@dfm if you let me know which approach is your favorite I'll give it a go.

@dfm
Copy link
Owner

dfm commented Jan 13, 2022

Awesome!! I think the easiest first step would be the improved error message. What do you think?

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

No branches or pull requests

2 participants