Balanced neural ratio estimation #779
Conversation
There was a problem hiding this comment.
Awesome, thanks a lot for this!
One small comment below, but a few additional comments:
- could you add
BNREto thetestshere? - if you want, you can add BNRE to the documentation: README, API reference, landing page of the website, and tutorial on all available methods
| labels[1::2] = 0.0 | ||
|
|
||
| # Binary cross entropy to learn the likelihood (AALR-specific) | ||
| bce = nn.BCELoss()(likelihood, labels) |
There was a problem hiding this comment.
Is there a reason to not call super()._loss(theta, x)?
There was a problem hiding this comment.
Yes, I need access to the logits to compute the regularizer and the loss function does not return those.
There was a problem hiding this comment.
Makes sense, thank you!
|
Also, please run |
Codecov Report
@@ Coverage Diff @@
## main #779 +/- ##
==========================================
- Coverage 74.51% 74.42% -0.10%
==========================================
Files 77 78 +1
Lines 6118 6143 +25
==========================================
+ Hits 4559 4572 +13
- Misses 1559 1571 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
Hi Arnaud,
thanks for the documentation fixes. Could you still add BNRE to the tests and make sure that they pass?
Michael
|
Hi Michael, Yes sorry for the delay. I have not forgotten, it is my todo list and I will do it asap! |
|
Great, thanks! No rush from our side! |
|
Hi, I have now added the tests and updated the specifications to explain what BNRE is. I believe that I have addressed all the comments, tell me if there is something missing! |
Implement Balanced neural ratio estimation.