Skip to content

Conversation

@yf225
Copy link
Contributor

@yf225 yf225 commented Jul 13, 2018

test_neg sometimes fails internally because random_() can generate an out-of-range value for CharTensor. This PR fixes it.

@yf225 yf225 requested a review from soumith July 13, 2018 20: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.

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

@colesbury
Copy link
Member

I'm OK with this if we need to fix the tests immediately, but shouldn't we fix random_() on CharTensor instead?

I don't understand the strategy of enabling overflow tests, but trying to avoid triggering them in test code. If we're OK with overflows then they shouldn't break tests. If we're not OK, then we shouldn't try to avoid testing broken code.

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.

int8_t tensors can represent numbers in range of [-128, 127], not [-128, 128]!

@yf225
Copy link
Contributor Author

yf225 commented Jul 16, 2018

@apaszke random_(A, B) seems to be picking number from range [A, B)? e.g. torch.CharTensor(1).random_(0, 1) will always give 0, not 1.

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.

@yf225 is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

petrex pushed a commit to petrex/pytorch that referenced this pull request Jul 16, 2018
* upstream/master: (24 commits)
  Implement tensor weak references (pytorch#9363)
  Nuke TestCollectEnv (pytorch#9459)
  Add test case for segmentation fault fix in grad_fn (pytorch#9457)
  Add peephole optimization for type_as operators. (pytorch#9316)
  Fix out-of-range error for test_neg (pytorch#9431)
  add depthwise conv support for mkldnn (pytorch#8782)
  Refactor `_log_sum_exp` (pytorch#9173)
  Add ModuleDict and ParameterDict containers (pytorch#8463)
  Introduce SupervisedPtr, delete THAllocator and THCDeviceAllocator (pytorch#9358)
  Introducing IsInf (pytorch#9169)
  add device to CUDAEvent (pytorch#9415)
  Make localScalar error message more intuitive (pytorch#9443)
  Only accept continguous tensors in TopK for cuda (pytorch#9441)
  Add support for .norm() pytorch onnx export and ReduceL1/ReduceL2 caffe2 operators (pytorch#9299)
  Only view() rhs of index_put if we need to (pytorch#9424)
  Add BatchBucketizeOp in caffe2 (pytorch#9385)
  Implementation of Wngrad optimizer caffe2 python wrapper and unit test on least square regression (pytorch#9001)
  Implementation and operator test for Wngrad optimizer (pytorch#8999)
  Fix segmentation fault in grad_fn (pytorch#9292)
  update docs (pytorch#9423)
  ...
goldsborough pushed a commit to goldsborough/pytorch that referenced this pull request Jul 20, 2018
Summary:
`test_neg` sometimes fails internally because `random_()` can generate an out-of-range value for CharTensor. This PR fixes it.
Pull Request resolved: pytorch#9431

Reviewed By: SsnL

Differential Revision: D8843284

Pulled By: yf225

fbshipit-source-id: bf516cceb8f780e133fa54f7364c77821eb7c013
jramseyer pushed a commit to jramseyer/pytorch that referenced this pull request Jul 30, 2018
Summary:
`test_neg` sometimes fails internally because `random_()` can generate an out-of-range value for CharTensor. This PR fixes it.
Pull Request resolved: pytorch#9431

Reviewed By: SsnL

Differential Revision: D8843284

Pulled By: yf225

fbshipit-source-id: bf516cceb8f780e133fa54f7364c77821eb7c013
goodlux pushed a commit to goodlux/pytorch that referenced this pull request Aug 15, 2018
Summary:
`test_neg` sometimes fails internally because `random_()` can generate an out-of-range value for CharTensor. This PR fixes it.
Pull Request resolved: pytorch#9431

Reviewed By: SsnL

Differential Revision: D8843284

Pulled By: yf225

fbshipit-source-id: bf516cceb8f780e133fa54f7364c77821eb7c013
@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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants