probnum icon indicating copy to clipboard operation
probnum copied to clipboard

Bq test problems

Open fxbriol opened this issue 3 years ago • 9 comments

In a Nutshell

This pull request includes a number of test functions to benchmark numerical integration methods.

This includes test functions for integration against the Lebesgue measure on [0,1]^d, test functions for integration against multivariate Gaussian on R^d, and a function which transforms test functions for integration against Lebesgue measure on [0,1]^d to integrands against multivariate Gaussians on R^d

fxbriol avatar Dec 06 '21 14:12 fxbriol

@JonathanWenger @marvinpfoertner

fxbriol avatar Dec 06 '21 14:12 fxbriol

Also adding @mmahsereci since you were part of the discussion in the other pull request.

fxbriol avatar Dec 06 '21 14:12 fxbriol

Dear @marvinpfoertner , @mmahsereci : thanks both for the suggestions - they are really appreciated as I am very much a beginner when it comes to this. I'll have a look at how to address them early next week.

F-X

fxbriol avatar Dec 08 '21 14:12 fxbriol

Very nice 👍🏻 I like the exhaustive docstrings!

A few general comments:

  • [ ] I would suggest providing QuadratureProblem "constructors" instead of the individual integrand and integral functions. This makes it easier for the user to use the problems later on. I provided a proposal implementation for the genz_continuous problem.
  • [ ] Input checking should not be done via assertions, since these are just meant for "sanity checking" and can be turned of in the interpreter configuration. User-facing functions should instead throw a meaningful exception describing the problem.
  • [ ] function names should generally be snake_case and lowercase
  • [ ] The .DS_Store files need to be removed. These are macOS system files which should never be checked into a git repo. You can generally ignore these files by adding a global gitignore file in your home directory

All done! Thanks again for those comments, very helpful.

fxbriol avatar Dec 16 '21 19:12 fxbriol

Hi @marvinpfoertner , @mmahsereci : Thanks again for your comments. I think I have now managed to implement all of them (I will certainly keep the one about keeping pull requests short in mind for the future!). I am still struggling to pass all of the checks, and was wondering if you could give me a hand with this?

Also to answer to Maren's point about Monte Carlo tests more directly: I have now added seeds. The integrands all also work with bayesquad, but I have found the bayesquad function not be very reliable which is why I do the tests with MC. I think the main reason is that we do not estimate hyper parameters, and as a result BQ is over confident and the stopping rule becomes inaccurate.

fxbriol avatar Dec 16 '21 19:12 fxbriol

Hi @fxbriol,

I am still struggling to pass all of the checks, and was wondering if you could give me a hand with this?

Sure! I'll have a look at this tomorrow afternoon and ping you once I'm done.

marvinpfoertner avatar Dec 20 '21 14:12 marvinpfoertner

Also to answer to Maren's point about Monte Carlo tests more directly: I have now added seeds. The integrands all also work with bayesquad, but I have found the bayesquad function not be very reliable which is why I do the tests with MC. I think the main reason is that we do not estimate hyper parameters, and as a result BQ is over confident and the stopping rule becomes inaccurate.

Sure that makes sense. What I was referring to initially was to make sure that no errors are thrown, e.g., shape errors, when the Callable you define (the test function) is used with the bayesquad interface. It's just a manual check if all components work/run well together. Ideally these checks are done automatically, and are called end-to-end integration tests (the word 'integration' has a different meaning here), but we don't have them yet in probnum (at least for quad package), so it's good to do a quick manual check. The kernel embedding values (if the integral is correct) are tested separately already. The BQ code should not be used to check integrals as you say :)

You can also have a look here and here where Gauss-Hermite tensor product rule is used to test integrals. May be an alternative to MC sampling and we could re-use the existing code.

@marvinpfoertner If the QuadratureProblem needs some re-design in this PR, that should be OK. It was defined a bit pre-maturely before the bayesquad interface or any test problem in fact was defined. For instance, it might be easier to use domain attribute instead of bounds. But we can also change that at a later point.

mmahsereci avatar Dec 21 '21 10:12 mmahsereci

Codecov Report

Merging #585 (434a781) into main (cb5b55a) will decrease coverage by 0.10%. The diff coverage is 86.97%.

:exclamation: Current head 434a781 differs from pull request most recent head 86a9ba3. Consider uploading reports for the commit 86a9ba3 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #585      +/-   ##
==========================================
- Coverage   90.02%   89.92%   -0.11%     
==========================================
  Files         194      197       +3     
  Lines        7350     7611     +261     
  Branches     1161     1232      +71     
==========================================
+ Hits         6617     6844     +227     
- Misses        489      523      +34     
  Partials      244      244              
Impacted Files Coverage Δ
...robnum/problems/zoo/quad/_quadproblems_gaussian.py 20.93% <20.93%> (ø)
src/probnum/problems/zoo/quad/__init__.py 100.00% <100.00%> (ø)
...probnum/problems/zoo/quad/_quadproblems_uniform.py 100.00% <100.00%> (ø)

codecov[bot] avatar Dec 21 '21 16:12 codecov[bot]

Hi @fxbriol :) What is the status of your PR? Should we maybe set up a meeting soon to discuss how we finalize this?

marvinpfoertner avatar Feb 25 '22 17:02 marvinpfoertner

Closing due to inactivity. Feel free to re-open in case you want to finish the PR at some point @fxbriol .

mmahsereci avatar Feb 22 '23 15:02 mmahsereci