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

998 abc methods for trial based data using statistical distances #1104

Conversation

theogruner
Copy link
Contributor

@theogruner theogruner commented Mar 22, 2024

What does this implement/fix? Explain your changes

Adding an approximation of the squared 2-Wasserstein distance based on Sinkhorn iterations as an additional statistical distance to the available metrics. Furthermore, extending MCABC and SMCABC to allow conditioning on multiple observations using statistical distances.

Does this close any currently open issues?

Fixes #998

Checklist

  • I have read and understood the contribution
    guidelines
  • I agree with re-licensing my contribution from AGPLv3 to Apache-2.0.
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • I have reported how long the new tests run and potentially marked them
    with pytest.mark.slow.
  • New and existing unit tests pass locally with my changes
  • I performed linting and formatting as described in the contribution
    guidelines
  • I rebased on main (or there are no conflicts with main)

Copy link
Contributor

@janfb janfb left a comment

Choose a reason for hiding this comment

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

Thanks a lot, this is great!
Added a couple of comments.

sbi/inference/abc/abc_base.py Outdated Show resolved Hide resolved
sbi/inference/abc/abc_base.py Outdated Show resolved Hide resolved
sbi/inference/abc/abc_base.py Outdated Show resolved Hide resolved
sbi/inference/abc/abc_base.py Outdated Show resolved Hide resolved
sbi/inference/abc/abc_base.py Outdated Show resolved Hide resolved
sbi/utils/metrics.py Show resolved Hide resolved
tests/abc_test.py Outdated Show resolved Hide resolved
tests/abc_test.py Show resolved Hide resolved
tests/abc_test.py Show resolved Hide resolved
tests/metrics_test.py Show resolved Hide resolved
@theogruner theogruner requested a review from janfb March 29, 2024 17:37
…hods-for-trial-based-data-using-statistical-distances
@theogruner theogruner marked this pull request as ready for review April 2, 2024 19:40
Copy link
Contributor

@janfb janfb left a comment

Choose a reason for hiding this comment

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

Great, thanks for the edits!

I added some last comments, once those addressed, the PR can be merged 🎉

sbi/inference/abc/abc_base.py Show resolved Hide resolved
sbi/inference/abc/distances.py Outdated Show resolved Hide resolved
sbi/inference/abc/distances.py Outdated Show resolved Hide resolved
sbi/inference/abc/distances.py Outdated Show resolved Hide resolved
sbi/inference/abc/distances.py Outdated Show resolved Hide resolved
sbi/inference/abc/distances.py Outdated Show resolved Hide resolved
sbi/inference/abc/distances.py Outdated Show resolved Hide resolved
sbi/inference/abc/distances.py Outdated Show resolved Hide resolved
@janfb janfb self-assigned this Apr 4, 2024
@janfb
Copy link
Contributor

janfb commented Apr 12, 2024

Ping @theogruner 🙂 We are so close to merging here.

@janfb
Copy link
Contributor

janfb commented Apr 24, 2024

Ping @theogruner will you have time to finish this soon?
Please let me know if not, so that we can go ahead for the release - thanks 🙏

Copy link

codecov bot commented May 28, 2024

Codecov Report

Attention: Patch coverage is 17.41935% with 128 lines in your changes are missing coverage. Please review.

Project coverage is 72.94%. Comparing base (1b268b8) to head (ea43224).

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1104       +/-   ##
===========================================
- Coverage   83.08%   72.94%   -10.14%     
===========================================
  Files          92       93        +1     
  Lines        7259     7371      +112     
===========================================
- Hits         6031     5377      -654     
- Misses       1228     1994      +766     
Flag Coverage Δ
unittests 72.94% <17.41%> (-10.14%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
sbi/inference/abc/abc_base.py 34.14% <66.66%> (-59.41%) ⬇️
sbi/inference/abc/mcabc.py 14.51% <0.00%> (-67.63%) ⬇️
sbi/inference/abc/smcabc.py 11.30% <4.76%> (-69.81%) ⬇️
sbi/inference/abc/distances.py 28.33% <28.33%> (ø)
sbi/utils/metrics.py 38.98% <11.29%> (-17.12%) ⬇️

... and 19 files with indirect coverage changes

@theogruner
Copy link
Contributor Author

Hi,
I'm really sorry for the huge delay in my response and for prolonging the release, especially given the minimal changes required.
I have now made the necessary updates and hope everything can be merged now :)
Best, Theo

@theogruner theogruner requested a review from janfb May 28, 2024 19:57
Copy link
Contributor

@janfb janfb left a comment

Choose a reason for hiding this comment

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

Great, looks good now! Thanks a lot for adding these features 👏

@janfb janfb changed the title 998 improving abc methods for trial based data using statistical distances 998 abc methods for trial based data using statistical distances Jun 3, 2024
@janfb janfb merged commit 7900af0 into main Jun 3, 2024
5 of 7 checks passed
@janfb janfb deleted the 998-improving-abc-methods-for-trial-based-data-using-statistical-distances branch June 3, 2024 13:49
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.

Improving ABC methods for trial-based data using statistical distances
2 participants