Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Decouple dtype from shape for Random multinomial #15980

Merged
merged 4 commits into from
Aug 25, 2019

Conversation

ChaiBapchya
Copy link
Contributor

@ChaiBapchya ChaiBapchya commented Aug 23, 2019

Description

Currently, if we pass shape >2**32 to random multinomial
It fails with this error
dtype' does not have a sufficient precision to represent the indices of the input array.

It's wrong for an operator to expect the shape to fall within the limits of dtype. Reason - they are not related.

even if default dtype is float32, it shouldn't be restrict the shape space to 2**32.

======================================================================
ERROR: test_large_vector.test_ndarray_random_multinomial
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/ubuntu/anaconda3/lib/python3.6/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/ubuntu/workspace/lts/vector/incubator-mxnet/tests/python/unittest/common.py", line 177, in test_new
    orig_test(*args, **kwargs)
  File "/home/ubuntu/workspace/lts/vector/incubator-mxnet/tests/nightly/test_large_vector.py", line 67, in test_ndarray_random_multino$
ial
    a = nd.random.multinomial(nd.random.uniform(shape=(LARGE_X)))
  File "/home/ubuntu/workspace/lts/vector/incubator-mxnet/python/mxnet/ndarray/random.py", line 562, in multinomial
    return _internal._sample_multinomial(data, shape, get_prob, out=out, dtype=dtype, **kwargs)
  File "<string>", line 70, in _sample_multinomial
  File "/home/ubuntu/workspace/lts/vector/incubator-mxnet/python/mxnet/_ctypes/ndarray.py", line 100, in _imperative_invoke
    ctypes.byref(out_stypes)))
  File "/home/ubuntu/workspace/lts/vector/incubator-mxnet/python/mxnet/base.py", line 254, in check_call
    raise MXNetError(py_str(_LIB.MXGetLastError()))
mxnet.base.MXNetError: [20:02:50] ../src/operator/random/./sample_multinomial_op.h:76: Check failed: ishape[ishape.ndim() - 1] <= mxne$
::common::MaxIntegerValue<DType>() (5000000000 vs. 2147483647) : 'dtype' does not have a sufficient precision to represent the indices
of the input array.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Code is well-documented:
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

Removed the check for shape to be within the MAX INT limit

Comments

Must be made to ensure shape and dtype are decoupled.

Copy link
Contributor

@apeforest apeforest left a comment

Choose a reason for hiding this comment

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

LGTM

@marcoabreu
Copy link
Contributor

Test?

@ChaiBapchya
Copy link
Contributor Author

ChaiBapchya commented Aug 24, 2019

I tested locally for
https://github.com/apache/incubator-mxnet/blob/9bf8f7fa064b1000406f3cc580e13209295f8b7e/tests/nightly/test_large_vector.py#L66

from #15941
Didn't want to duplicate test hence didn't add again here.
And for backward compatibility, tested if it passes all the unit tests using nosetest

@marcoabreu
Copy link
Contributor

Ok so this fixes a broken nightly test case?

@ChaiBapchya
Copy link
Contributor Author

@marcoabreu It doesn't fix a broken nightly test. The issue we encountered while writing nightly test for large vector (2**32)

Currently, behavior of random.multinomial is to expect shape and dtype to agree.
For eg if dtype is given as float16, shape has to be within max integer limit

>>> mx.nd.random.multinomial(mx.nd.random.uniform(shape=65536),dtype="float16")
Check failed: ishape[ishape.ndim() - 1] <= mxnet::common::MaxIntegerValue<DType>() (65536 vs. 2048) : 'dtype' does not have a sufficient precision to represent the indices of the input array
>>> mx.nd.random.multinomial(mx.nd.random.uniform(shape=4294967296),dtype="float32")
Check failed: ishape[ishape.ndim() - 1] <= mxnet::common::MaxIntegerValue<DType>() (4294967296 vs. 16777216) : 'dtype' does not have a sufficient precision to represent the indices of the input array.

@apeforest apeforest merged commit 3b7e484 into apache:master Aug 25, 2019
@ChaiBapchya ChaiBapchya deleted the random_multinomial branch August 25, 2019 22:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants