marcoabreu commented on a change in pull request #16570: [WIP]Showing proper
error message when an attempt is made to create large tensor but MXNet is not
built with it
URL: https://github.com/apache/incubator-mxnet/pull/16570#discussion_r339275852
##########
File path: python/mxnet/ndarray/ndarray.py
##########
@@ -155,6 +155,13 @@ def _new_alloc_handle(shape, ctx, delay_alloc,
dtype=mx_real_t):
ctypes.c_int(int(_DTYPE_NP_TO_MX[np.dtype(dtype).type])),
ctypes.byref(hdl)))
else:
+ size = 1
+ for idx in shape:
+ size = size * idx
+ if size > 2**31:
+ raise Exception("[_new_alloc_handle] Size of tensor you are trying
to allocate is " +
Review comment:
Didn't we have 32 and 64 bit C api functions?
It sounds counter intuitive initially, but if we have overloaded c api
functions we could always expose them. If we're compiled without 64bit Support,
the c api would then return an error instead of passing on the request. IMO
this design makes the typing a bit more explicit and transparent (while I have
to admit that I generally dislike the verbose explicity in c++) instead of
hiding all the magic in a simple typedef override and some preprocessor
statements.
The logic in the frontend would then be to call the 32bit c api functions
with small tensors and 64bit one with large tensors. Of course it needs some
tweaking because that's not a fail proof design against overflow of the
intermediary results.
What it seems like to me is that a C Api is supposed to be a static contract
that is always present. But with the 64bit Support we're now introducing
versions of the c api that depend on the compile options. This then breaks the
exchangability and decoupling. In the end it's a matter of taste, but I think
we should uphold ourselves to a standard where the c api never changes but the
backend might behave differently depending on the chosen options. At the same
time, we might want to make it a rule that c api function definitions must not
be conditional.
It's a bit derailing, but what do you think about that idea?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
