Fix issue with sampling without replacement#386
Fix issue with sampling without replacement#386aristizabal95 wants to merge 3 commits intorusty1s:masterfrom
Conversation
Before this fix uniform sampling is not guaranteed, over-representing the first combination of neighbors, and not sampling the last possible combination Co-authored-by: Jaime Mosquera Gutiérrez <Jaimemosg@users.noreply.github.com> Co-authored-by: Andres Uriza <pa-uriza@users.noreply.github.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #386 +/- ##
=======================================
Coverage 73.09% 73.09%
=======================================
Files 29 29
Lines 1171 1171
=======================================
Hits 856 856
Misses 315 315 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This pull request had no activity for 6 months. It will be closed in 2 weeks unless there is some new activity. |
|
We believe this is still relevant. @rusty1s let us know if you have any thoughts on this |
|
This pull request had no activity for 6 months. It will be closed in 2 weeks unless there is some new activity. |
|
@rusty1s hope you get a chance to review this sometime. We still believe this is relevant |
Before this fix uniform sampling is not guaranteed, over-representing the first combination of neighbors, and not sampling the last possible combination.
This PR ensures all neighbor combinations are considered during sampling, by extending the range of possible indices to include the last element.
Referencing to the example showcased in #385, using this change the sampling distribution is now:
Closes #385.