-
Notifications
You must be signed in to change notification settings - Fork 197
Score-based iid sampling #1381
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
Score-based iid sampling #1381
Conversation
Okey, everything should be implemented now. This acutally became quite a big PR now. A few more points:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1381 +/- ##
===========================================
- Coverage 89.38% 34.65% -54.74%
===========================================
Files 119 121 +2
Lines 8905 9338 +433
===========================================
- Hits 7960 3236 -4724
- Misses 945 6102 +5157
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This is now basically done. With the review, one should probably wait until the other score branch and type fixes are merged. But the major changes are:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, great effort! 👏 Thanks for adding all those methods!
Looks great overall, but I was a bit confused by the class structure in the score_fn_iid.py
and added a couple of comments.
Also, the tests can be refactored a bit.
Please note that you might have to rebase or merge again with main
once #1404 is merged.
Thanks for the review. I addressed most of the points above and left open what I wasn't sure about. Summarizing points:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update @manuelgloeckler, almost done!
Added a couple of final questions and pushed small docs fixes myself.
all CD tests are passing! |
…into 1226-score-based-iid
There was a problem hiding this 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 update!
fixed some typos and will update the changelog to include this into release.
Completes the missing features based on score estimation #1226.