Uniform integration measure for BQ #176
Conversation
Codecov Report
@@ Coverage Diff @@
## master #176 +/- ##
==========================================
- Coverage 90.05% 89.12% -0.93%
==========================================
Files 99 100 +1
Lines 2583 2648 +65
Branches 255 264 +9
==========================================
+ Hits 2326 2360 +34
- Misses 214 244 +30
- Partials 43 44 +1
Continue to review full report at Codecov.
|
|
Some additional explanation (to avoid confusion): The PR is also supposed to ensure that whenever the integration bounds are changed, it does so consistently, which has not been the case before. This is why there are some changes in other objects that use the integral bounds. There are also some tests which check if integral bounds are re-set correctly. All this was needed because the integration over the uniform measure is very easy to do by changing the integral bounds temporarily. |
|
|
||
| # setting new bounds also checks bound validity | ||
| self.integral_bounds = new_integral_bound_list | ||
| integral_mean_lebesgue, integral_var_lebesgue = self._integrate_lebesgue() |
There was a problem hiding this comment.
The other approach would be to input integration_bounds to self._integrate_lebesgue() and then into all kernel methods that require the bounds instead of having the bounds as an attribute on the kernel. This would avoid needing to worry about the integration bounds setting and resetting. Thoughts?
There was a problem hiding this comment.
I see your point, but I think that might cause bigger problems because then you need to make sure that whoever designs any integration method use the same bounds for all the methods of the kernel, which is very error prone. Having the attribute in the kernel (the only place where integration happens) avoids that. I'd rather make sure to write a nice setter.
Also, to be clear, I do not want the user to change the integral bounds. They are the parameter space and I see no point in changing them. It is just a way to internally do the uniform measure.
Issue #130 , if available:
Description of changes:
Structure for vanilla BQ integration against some measure. This PR only contains the uniform measure for simplicity. Gaussian will come later.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.