Incubator-mxnet: MaxIntegerValue is buggy

[larroy]

Description

This code doesn't do what it claims, and does it wrong. Just filing an issue so I don't forget to add a fix.

https://github.com/apache/incubator-mxnet/blob/master/src/common/utils.h#L666

[vrakesh]

@mxnet-label-bot add [Bug]

[apeforest]

@mxnet-label-bot Add [Call for Contribution, Good First Issue]

[kshitij12345]

Hi,
Sorry this may be a lame question. But what is wrong with this function. When I tried to see where this function is used, it was always used with Template Parameter and couldn't figure exactly which types is the function used with. So I tried it with signed and unsigned

• char
• short
• int
• long

and

• float
• double

Got what I expected.
However didn't try the mshadow::half::halt_t case.
I would be interested to know more and also help with resolution of this issue.

Thank You.

[ChaiBapchya]

Hey @kshitij12345
Looks like a valid question. Maybe @larroy can weigh in.
Also doing some git blame brought up this PR https://github.com/apache/incubator-mxnet/pull/10970 which might help in further deep dive.

[larroy]

I will have a look at this when I have some time. Thanks for looking at this issue.

[piyushghai]

I don't think this is a bug. This is more of a question.

@mxnet-label-bot Update [Question, Good first issue, Call for contribution]

[ChaiBapchya]

@larroy I found MaxIntegerValue used in sample_multinomial_op.h incorrectly.
For example

>>> 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.

Maybe this can facilitate discussion.