Skip to content

Conversation

@ailzhang
Copy link
Contributor

Update documentation to raise awareness of the fix in #12490. Thanks @matteorr for pointing this out!

:attr:`input` length (or number of columns of :attr:`input` if it is a matrix).
.. note::
When drawn without replacement, :attr:`num_samples` must be lower than
number of non-zero elements in :attr:`input` (or the min number of non-zero
Copy link
Collaborator

Choose a reason for hiding this comment

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

The example below is still outdated.

Copy link
Collaborator

@ssnl ssnl left a comment

Choose a reason for hiding this comment

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

All good except for 1 nit

tensor([ 1, 2, 0, 0])
>>> torch.multinomial(weights, 2)
tensor([1, 2])
>>> torch.multinomial(weights, 4) # ERROR!
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd add a few words explaining the error. Adding the error message here would also be useful.

@ailzhang ailzhang force-pushed the update_multinomial_doc branch from 39a46e6 to 83291b5 Compare February 19, 2019 19:52
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.

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

ezyang pushed a commit to ezyang/pytorch that referenced this pull request Feb 19, 2019
Summary:
Update documentation to raise awareness of the fix in pytorch#12490. Thanks matteorr for pointing this out!
Pull Request resolved: pytorch#17269

Reviewed By: ezyang

Differential Revision: D14138421

Pulled By: ailzhang

fbshipit-source-id: 6433f9807a6ba1d871eba8e9d37aa6b78fa1e1fd
@ezyang ezyang added the merged label Jun 25, 2019
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.

4 participants