"metric" is linear but "metric_bounds" is log
arodland opened this issue · 3 comments
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.
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.
Awesome!! I think the easiest first step would be the improved error message. What do you think?