[Numpy] Basic indexing in symbolic interface of DeepNumpy #16621

sxjscience · 2019-10-25T05:56:31Z

Description

Continue #15905.

Add support of basic symbolic indexing, currently we support the following indexing types
- integer types
- python slice
- tuple of the above two types for multi-axis slicing
Fixes the MKLDNN integration of MXNet, which does not support 0-size NDArray. Currently, the solution is to disable MKLDNN if the size of input is 0.
Support loading and saving symbols in the Numpy interface
Add two testing utility functions:
- check_hybridize_consistency, this is added to test_utils
- check_gluon_save_load

Once merged, this will fix #16279 and #16887

Followup work will be to support Ellipsis in the symbolic interface.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
Changes are complete (i.e. I finished coding on this PR)
All changes have test coverage:
Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
Code is well-documented:
For user-facing API changes, API doc string has been updated.
For new C++ functions in header files, their functionalities and arguments are documented.
For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
Check the API doc at https://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

Numpy Symbolic Basic Indexing, tests
MKLDNN integration fix
Save and load numpy symbols
Testing utilities for checking the consistency of hybridblock

Change the error raised in _op.split Fix sanity issues

update Update _symbol.py fix Update _symbol.py fix Update symbol.py Update symbol.py update update try to fix fix bug Fix for hsplit and split fix try to fix try to fix try to fix try to fix fix bug fix lint fix slicing fix concatenate try to not use output_is_list Update test_numpy_gluon.py try to fix the basic indexing try to fix fix fix debug fix bug debug debug debug try to fix fix skip the zero-size ndarrays in MKLDNN Update multiarray.py fix bug Update ndarray.py remove debug fix bug try to fix Update test_numpy_gluon.py fix add back Update test_numpy_gluon.py Update test_numpy_gluon.py fix bug try to fix fix test

Test for case2 fix lint fix accerate testing move check_gluon_hybridize_consistency to test_utils update docstring Use Size in the implementation fix bug

ptrendx · 2019-11-25T18:42:44Z

@sxjscience Issue #16887 is tagged as 1.6, but this PR introduces new functionality (so probably we do not want it in 1.6). Are you going to make another PR for 1.6 branch with the targeted fix for #16887 or should we drop 1.6 tag from that issue?

sxjscience · 2019-11-25T18:47:40Z

@ptrendx Is it possible to include this into 1.6? It's more like a bug-fix because in the previous version, we haven't checked the consistency of hybridization.

sxjscience · 2019-11-25T19:25:36Z

@ptrendx After a second thought, I think it would be better to fix the bug first. Let me extract part of this PR to a bugfix.

reminisce · 2019-11-25T22:49:08Z

python/mxnet/symbol/numpy/_symbol.py

+            The name of this symbol, returns ``None`` for list symbol.
+        """
+        if self.num_outputs > 1:
+            return None


raise instead of return?

reminisce · 2019-11-25T23:02:12Z

python/mxnet/ndarray/ndarray.py

@@ -3125,6 +3128,26 @@ def _get_dim_size(start, stop, step):
    return dim_size


+def _get_slice_len_for(slc, seq_length):


nit: change _get_slice_len_for to _get_slice_len.

Let me fix it in the other PR (#16902)

python/mxnet/ndarray/numpy/_op.py

python/mxnet/symbol/numpy/_symbol.py

reminisce · 2019-11-25T23:10:33Z

python/mxnet/symbol/numpy/_symbol.py

+    --------
+    _Symbol.tojson : Used to save symbol into json string.
+    """
+    if not isinstance(json_str, string_types):


Reuse mx.sym.load_json?

reminisce · 2019-11-25T23:11:13Z

python/mxnet/symbol/numpy/_symbol.py

+    --------
+    Symbol.save : Used to save symbol into file.
+    """
+    if not isinstance(fname, string_types):


Reuse mx.sym.load?

I've copied the code here because we need to return the _Symbol object.

Mike Mao and others added 2 commits October 24, 2019 22:33

add symbolic indexing, waiting for split and concatenate to be merged

7644e4f

Change the error raised in _op.split Fix sanity issues

Merge remote-tracking branch 'upstream/master' into mikemwx-local

b3e00c1

sxjscience requested a review from szha as a code owner October 25, 2019 05:56

reminisce added the Numpy label Oct 25, 2019

Merge remote-tracking branch 'upstream/master' into mikemwx-local

1d5d1f6

szha mentioned this pull request Nov 4, 2019

[Numpy] Basic indexing in symbolic interface #15905

Closed

5 tasks

Merge remote-tracking branch 'upstream/master' into mikemwx-local

03c343c

sxjscience mentioned this pull request Nov 19, 2019

[Numpy] The argument parser of some operators cannot parse numpy integers or mx.numpy integers correctly. #16855

Open

sxjscience force-pushed the mikemwx-local branch from e8365e8 to f299ad0 Compare November 24, 2019 07:35

Merge remote-tracking branch 'upstream/master' into mikemwx-local

b499d97

sxjscience changed the title ~~[Numpy][WIP] Basic indexing in symbolic interface~~ [Numpy] Basic indexing in symbolic interface Nov 24, 2019

sxjscience requested a review from reminisce November 24, 2019 07:48

sxjscience force-pushed the mikemwx-local branch from 258aab9 to a670f97 Compare November 24, 2019 08:05

sxjscience mentioned this pull request Nov 24, 2019

[Bug] Inconsistency between HybridBlock and Block #16279

Closed

sxjscience changed the title ~~[Numpy] Basic indexing in symbolic interface~~ [Numpy] Basic indexing in symbolic interface of DeepNumpy Nov 24, 2019

sxjscience force-pushed the mikemwx-local branch from 2efbf1d to 232052c Compare November 24, 2019 20:21

fix lint

3c67b8f

Test for case2 fix lint fix accerate testing move check_gluon_hybridize_consistency to test_utils update docstring Use Size in the implementation fix bug

sxjscience force-pushed the mikemwx-local branch from 226f928 to 3c67b8f Compare November 24, 2019 21:57

reminisce reviewed Nov 25, 2019

View reviewed changes

sxjscience added 4 commits November 25, 2019 16:40

Address comments

03c5f54

Update _symbol.py

20bfd31

add load_json

1aa31a6

address comment

53fbf92

reminisce approved these changes Nov 26, 2019

View reviewed changes

Merge remote-tracking branch 'upstream/master' into mikemwx-local

c2cad11

sxjscience merged commit a98cefc into apache:master Nov 27, 2019

sxjscience deleted the mikemwx-local branch December 3, 2019 01:40

zheyuye mentioned this pull request Jan 15, 2020

np.ndarray indexing after hybridize #17327

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Numpy] Basic indexing in symbolic interface of DeepNumpy #16621

[Numpy] Basic indexing in symbolic interface of DeepNumpy #16621

sxjscience commented Oct 25, 2019 •

edited

Loading

ptrendx commented Nov 25, 2019

sxjscience commented Nov 25, 2019

sxjscience commented Nov 25, 2019

reminisce Nov 25, 2019

reminisce Nov 25, 2019

sxjscience Nov 26, 2019

reminisce Nov 25, 2019

reminisce Nov 25, 2019

sxjscience Nov 26, 2019

		@@ -3125,6 +3128,26 @@ def _get_dim_size(start, stop, step):
		return dim_size


		def _get_slice_len_for(slc, seq_length):

[Numpy] Basic indexing in symbolic interface of DeepNumpy #16621

[Numpy] Basic indexing in symbolic interface of DeepNumpy #16621

Conversation

sxjscience commented Oct 25, 2019 • edited Loading

Description

Checklist

Essentials

Changes

ptrendx commented Nov 25, 2019

sxjscience commented Nov 25, 2019

sxjscience commented Nov 25, 2019

reminisce Nov 25, 2019

Choose a reason for hiding this comment

reminisce Nov 25, 2019

Choose a reason for hiding this comment

sxjscience Nov 26, 2019

Choose a reason for hiding this comment

reminisce Nov 25, 2019

Choose a reason for hiding this comment

reminisce Nov 25, 2019

Choose a reason for hiding this comment

sxjscience Nov 26, 2019

Choose a reason for hiding this comment

sxjscience commented Oct 25, 2019 •

edited

Loading