Skip to content

allow RandomSampler to sample with replacement#9911

Closed
weiyangfb wants to merge 3 commits intopytorch:masterfrom
weiyangfb:RamdomSampler_with_Replacement
Closed

allow RandomSampler to sample with replacement#9911
weiyangfb wants to merge 3 commits intopytorch:masterfrom
weiyangfb:RamdomSampler_with_Replacement

Conversation

@weiyangfb
Copy link
Contributor

fixes #7908

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@weiyangfb has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@weiyangfb has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@weiyangfb
Copy link
Contributor Author

cc @apaszke @ssnl can I get a review on this PR?

@weiyangfb weiyangfb added the ready for review (this tag is deprecated) All PRs are ready for review unless they are draft, WIP, or have undismissed requested changes label Aug 1, 2018
Copy link
Contributor

@apaszke apaszke left a comment

Choose a reason for hiding this comment

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

Test needs a fix or it will be flaky. Apart from that LGTM.

count_repeated = 0
for val in bin.values():
if val > 1:
count_repeated += 1

This comment was marked as off-topic.

self.assertTrue(maxval < n)

sampler_without_replacement = RandomSampler(self.dataset)
count_repeated, minval, maxval = sample_stat(sampler_without_replacement, n)

This comment was marked as off-topic.

This comment was marked as off-topic.

n = len(self.dataset)
sampler_with_replacement = RandomSampler(self.dataset, n, True)
count_repeated, minval, maxval = sample_stat(sampler_with_replacement, n)
self.assertTrue(count_repeated > 0)

This comment was marked as off-topic.

This comment was marked as off-topic.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

weiyangfb has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@weiyangfb
Copy link
Contributor Author

@pytorchbot retest this please

@weiyangfb weiyangfb force-pushed the RamdomSampler_with_Replacement branch from f0e0af4 to d3f4e91 Compare August 27, 2018 07:37
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

weiyangfb has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

PenghuiCheng pushed a commit to PenghuiCheng/pytorch that referenced this pull request Sep 11, 2018
Summary:
fixes pytorch#7908
Pull Request resolved: pytorch#9911

Reviewed By: yf225

Differential Revision: D9023223

Pulled By: weiyangfb

fbshipit-source-id: 68b199bef3940b7205d0fdad75e7c46e6fe65ba7
@ezyang ezyang added the merged label Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review (this tag is deprecated) All PRs are ready for review unless they are draft, WIP, or have undismissed requested changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

why pytorch does not implement sampler with replacement(except weighted sampler)

4 participants