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

[Bug] Inconsistency between HybridBlock and Block #16279

Closed
sxjscience opened this issue Sep 25, 2019 · 8 comments · Fixed by #16621
Closed

[Bug] Inconsistency between HybridBlock and Block #16279

sxjscience opened this issue Sep 25, 2019 · 8 comments · Fixed by #16621
Assignees
Labels

Comments

@sxjscience
Copy link
Member

There are lots of operators that have inconsistent behaviors between hybridized and not-hybridized versions.

import mxnet as mx
from mxnet.gluon import HybridBlock

class Foo2(HybridBlock):
    def hybrid_forward(self, F, a):
        return a[0]


b1 = Foo2(prefix='hybridized')
b1.hybridize()
b2 = Foo2(prefix='no_hybrid')

out1 = b1(mx.nd.ones((10,)))
out2 = b2(mx.nd.ones((10,)))
print(out1)
print(out2)
[1. 1. 1. 1. 1. 1. 1. 1. 1. 1.]
<NDArray 10 @cpu(0)>

[1.]
<NDArray 1 @cpu(0)>

Even if we use the numpy interface, the following code cannot run:

import mxnet as mx
from mxnet.gluon import HybridBlock
mx.npx.set_np()


class Foo2(HybridBlock):
    def hybrid_forward(self, F, a):
        return a[0]


b1 = Foo2(prefix='hybridized')
b1.hybridize()
b2 = Foo2(prefix='no_hybrid')

out1 = b1(mx.np.ones((10,)))
out2 = b2(mx.np.ones((10,)))
print(out1)
print(out2)
      6 class Foo2(HybridBlock):
      7     def hybrid_forward(self, F, a):
----> 8         return a[0]
      9 
     10 

~/mxnet/python/mxnet/symbol/numpy/_symbol.py in __getitem__(self, key)
     49         num_outputs = _num_outputs(self)
     50         if num_outputs == 1:
---> 51             raise NotImplementedError
     52         if not isinstance(key, int):
     53             raise NotImplementedError

NotImplementedError: 
@mxnet-label-bot
Copy link
Contributor

Hey, this is the MXNet Label Bot.
Thank you for submitting the issue! I will try and suggest some labels so that the appropriate MXNet community members can help resolve it.
Here are my recommended label(s): Bug

@sxjscience sxjscience added the Bug label Sep 25, 2019
@reminisce
Copy link
Contributor

The problem is that ndarray indexing is not hybridizable. And more general, old symbol's __getitem__ semantics is not consistent with ndarray's. There is a PR trying to address this problem in numpy module, but needs more diving-deep review.
#15905

@sxjscience
Copy link
Member Author

@reminisce Thanks, we should fix these problems in the new numpy interface (also add the checks).

@reminisce
Copy link
Contributor

@sxjscience Agree. It's in the scope. 15905 tries to address the problem by differentiating indexing a list of ndarrays and a single ndarray. I'm afraid there may still be pitfalls/inconsistencies/incompatibility we have not considered. If you have time, could you review that PR, or we can discuss offline as well.

@sxjscience
Copy link
Member Author

sxjscience commented Sep 26, 2019 via email

@samskalicky
Copy link
Contributor

@zachgk assign [@sxjscience ]

@sxjscience
Copy link
Member Author

This will be solved once #16621 is merged. We have the following example:

import mxnet as mx
from mxnet.gluon import HybridBlock
mx.npx.set_np()

class Foo2(HybridBlock):
    def hybrid_forward(self, F, a):
        return a[0]


b1 = Foo2(prefix='hybridized')
b1.hybridize()
b2 = Foo2(prefix='no_hybrid')

out1 = b1(mx.np.ones((10,)))
out2 = b2(mx.np.ones((10,)))
print(out1)
print(out2)

Out:

1.0
1.0

@szha
Copy link
Member

szha commented Nov 28, 2019

To clarify, this inconsistency still exists in the NDArray and Symbol implementation and it's considered as "won't fix".

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants