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

Multilevel Bayesian quadrature #750

Merged
merged 46 commits into from
Feb 16, 2023

Conversation

tskarvone
Copy link
Collaborator

In a Nutshell

This RP adds a basic multilevel Bayesian quadrature method that is described in Proposition 1 of https://arxiv.org/pdf/2210.08329.pdf.

Detailed Description

  • This is essentially nothing more than a loop that calls bayesquad_from_data a number of times and sums the outputs.
  • For now the user is required to input the nodes and function evaluations, but it would be easy to include a version where a number of error tolerances that need to be met are provided instead. In that case the current method should probably be renamed e.g. bayesquad_multileve_from_data.
  • Testing seems somewhat difficult. Only input handling and trivial cases where the multilevel method reduces to traditional BQ are tested.

It may be that this should be reviewed only after some or all of #746, #748 and #749 are merged.

Related Issues

na

@tskarvone tskarvone self-assigned this Dec 9, 2022
Copy link
Contributor

@mmahsereci mmahsereci left a comment

Choose a reason for hiding this comment

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

What about multilevel_bayesquad_from_data as name? It would reflect the paper title.

In terms of tests, you can assert the type of the return objects and the length of the info list.

src/probnum/quad/_bayesquad.py Outdated Show resolved Hide resolved
src/probnum/quad/_bayesquad.py Outdated Show resolved Hide resolved
src/probnum/quad/_bayesquad.py Outdated Show resolved Hide resolved
src/probnum/quad/_bayesquad.py Outdated Show resolved Hide resolved
src/probnum/quad/_bayesquad.py Outdated Show resolved Hide resolved
src/probnum/quad/_bayesquad.py Show resolved Hide resolved
src/probnum/quad/_bayesquad.py Show resolved Hide resolved
src/probnum/quad/_bayesquad.py Outdated Show resolved Hide resolved
tskarvone and others added 7 commits December 14, 2022 15:40
Co-authored-by: Maren Mahsereci <42842079+mmahsereci@users.noreply.github.com>
Co-authored-by: Maren Mahsereci <42842079+mmahsereci@users.noreply.github.com>
Co-authored-by: Maren Mahsereci <42842079+mmahsereci@users.noreply.github.com>
Co-authored-by: Maren Mahsereci <42842079+mmahsereci@users.noreply.github.com>
@tskarvone
Copy link
Collaborator Author

What about multilevel_bayesquad_from_data as name? It would reflect the paper title.

I've renamed the method as you suggest.

@tskarvone
Copy link
Collaborator Author

All the failed checks are about src/probnum/quad/__init__.py::probnum.quad.multilevel_bayesquad_from_data but I do not understand what the problem is.

Copy link
Contributor

@mmahsereci mmahsereci left a comment

Choose a reason for hiding this comment

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

hey Toni, some more comments, mainly on the tests, but they should be all fast to fix.

The failing test was a doctest due to numerical imprecision. There is a comment on how to fix it. You can check doctests locally by running tox -e py3 -- src/probnum/quad/ in the root directory.

src/probnum/quad/_bayesquad.py Outdated Show resolved Hide resolved
src/probnum/quad/_bayesquad.py Outdated Show resolved Hide resolved
src/probnum/quad/_bayesquad.py Show resolved Hide resolved
src/probnum/quad/_bayesquad.py Outdated Show resolved Hide resolved
tests/test_quad/test_bayesquad/test_bq.py Outdated Show resolved Hide resolved
tests/test_quad/test_bayesquad/test_bq.py Outdated Show resolved Hide resolved
tests/test_quad/test_bayesquad/test_bq.py Outdated Show resolved Hide resolved
tests/test_quad/test_bayesquad/test_bq.py Outdated Show resolved Hide resolved
tests/test_quad/test_bayesquad/test_bq.py Outdated Show resolved Hide resolved
src/probnum/quad/_bayesquad.py Outdated Show resolved Hide resolved
tskarvone and others added 3 commits February 15, 2023 19:40
Co-authored-by: Maren Mahsereci <42842079+mmahsereci@users.noreply.github.com>
Co-authored-by: Maren Mahsereci <42842079+mmahsereci@users.noreply.github.com>
tskarvone and others added 16 commits February 15, 2023 19:48
Co-authored-by: Maren Mahsereci <42842079+mmahsereci@users.noreply.github.com>
Co-authored-by: Maren Mahsereci <42842079+mmahsereci@users.noreply.github.com>
Co-authored-by: Maren Mahsereci <42842079+mmahsereci@users.noreply.github.com>
Co-authored-by: Maren Mahsereci <42842079+mmahsereci@users.noreply.github.com>
Co-authored-by: Maren Mahsereci <42842079+mmahsereci@users.noreply.github.com>
Co-authored-by: Maren Mahsereci <42842079+mmahsereci@users.noreply.github.com>
Co-authored-by: Maren Mahsereci <42842079+mmahsereci@users.noreply.github.com>
Co-authored-by: Maren Mahsereci <42842079+mmahsereci@users.noreply.github.com>
Co-authored-by: Maren Mahsereci <42842079+mmahsereci@users.noreply.github.com>
Co-authored-by: Maren Mahsereci <42842079+mmahsereci@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Feb 15, 2023

Codecov Report

Merging #750 (c58dc7a) into main (a980df3) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head c58dc7a differs from pull request most recent head be1fa10. Consider uploading reports for the commit be1fa10 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #750      +/-   ##
==========================================
+ Coverage   91.16%   91.17%   +0.01%     
==========================================
  Files         215      215              
  Lines        7977     7993      +16     
  Branches     1015     1019       +4     
==========================================
+ Hits         7272     7288      +16     
  Misses        479      479              
  Partials      226      226              
Impacted Files Coverage Δ
src/probnum/quad/__init__.py 100.00% <100.00%> (ø)
src/probnum/quad/_bayesquad.py 100.00% <100.00%> (ø)

@tskarvone
Copy link
Collaborator Author

Thank you, Maren. That seems to have fixed most of the checks, though a Jupyter Notebook check still eludes me.

@mmahsereci mmahsereci requested a review from a team as a code owner February 16, 2023 11:50
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@mmahsereci
Copy link
Contributor

Thank you, Maren. That seems to have fixed most of the checks, though a Jupyter Notebook check still eludes me.

It doesn't seem to be related to the changes you made but a separate issue.

I made some cosmetic changes. Can you check if you're OK with it?

@mmahsereci
Copy link
Contributor

@JonathanWenger @marvinpfoertner As the jupyter test seems to be failing for other reasons (see #781 ) we'll merge this anyways if it's OK with everyone (once the rest of the tests pass of course.)

@tskarvone
Copy link
Collaborator Author

Thanks again, Maren. Your changes look OK. Happy to have this merged now.

@JonathanWenger
Copy link
Contributor

@JonathanWenger @marvinpfoertner As the jupyter test seems to be failing for other reasons (see #781 ) we'll merge this anyways if it's OK with everyone (once the rest of the tests pass of course.)

I just fixed this, see #783 . But merging this PR first is also fine with me.

@mmahsereci mmahsereci merged commit a668ccf into probabilistic-numerics:main Feb 16, 2023
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.

None yet

3 participants