[1.x] Backporting #18779 to v1.x #18894

samskalicky · 2020-08-10T20:26:55Z

Description

Backporting #18779 to v1.x

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 best of my knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

Feature1, tests, (and when applicable, API doc)
Feature2, tests, (and when applicable, API doc)

Comments

If this change is a backward incompatible change, why must this change be made.
Interesting edge cases to note here

mxnet-bot · 2020-08-10T20:26:56Z

Hey @samskalicky , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:

To trigger all jobs: @mxnet-bot run ci [all]
To trigger specific jobs: @mxnet-bot run ci [job1, job2]

CI supported jobs: [unix-gpu, website, centos-cpu, clang, miscellaneous, unix-cpu, windows-cpu, edge, centos-gpu, windows-gpu, sanity]

Note:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.

HahTK · 2020-08-10T21:09:44Z

Large scale cloud end users would need this feature in 1.8.
I propose we add tests for this feature for all 1.x APIs as part of this backport

HahTK · 2020-08-10T21:10:38Z

What is the timeline for this fix and MXNet 1.8 ?

Support additional inputs to custom subgraph ops that are not direct dependencies to ops in the subgraph. This will enable various use cases: custom control flow ops, custom ops that maintain a state that should be saved/loaded, etc. Highlights: * Added test that uses a graph pass (addInputPass) to add a new custom input to the subgraph op * Added new optional argument (clear) to hybridize & optimize_for APIs in Gluon Block to enable multiple optimizations * refactored lib_api.h JSON utilities * added new Graph data structure utilities to simplify custom graph passes * refactored custom op registration * enhanced custom subgraph op to support additional inputs to subgraph op that is not an input to ops in the subgraph * updated subgraph & graph pass READMEs * Added error messaging from external library

samskalicky · 2020-08-15T06:08:25Z

Large scale cloud end users would need this feature in 1.8.
I propose we add tests for this feature for all 1.x APIs as part of this backport

Hi @HahTK the PR on master was merged, and I cherry picked the commit into this branch. Notice that I left all the previous 1.x API tests (that were removed in the PR on master).

What is the timeline for this fix and MXNet 1.8 ?

merging this PR should be quick, get the CI to pass, one review from someone to check for missing things in 1.x and it should be good to go.

… subgraph_v18

samskalicky · 2020-08-16T18:25:31Z

I keep getting this weird Clojure failure:

[2020-08-16T17:41:29.383Z] lein test :only bert.bert-sentence-classification-test/train-test
[2020-08-16T17:41:29.383Z] 
[2020-08-16T17:41:29.383Z] FAIL in (train-test) (bert_sentence_classification_test.clj:90)
[2020-08-16T17:41:29.383Z] accuracy
[2020-08-16T17:41:29.383Z] expected: (< 0.5 (last (m/score model {:eval-data train-data, :eval-metric (eval-metric/accuracy)})))
[2020-08-16T17:41:29.383Z]   actual: (not (< 0.5 ##NaN))
[2020-08-16T17:41:29.383Z] WARN  org.apache.mxnet.DataDesc: Found Undefined Layout, will use default index 0 for batch axis

I did a rebase, but not sure why this is failing. None of my changes affect internal operators...

szha · 2020-08-16T18:50:51Z

You need the link fix from 7a84fe1

samskalicky · 2020-08-16T21:20:10Z

You need the link fix from 7a84fe1

Thanks! I created #18945 for the v1.x branch

… subgraph_v18

samskalicky · 2020-08-18T06:55:46Z

@HahTK @Kh4L Can you take a quick peek to make sure this PR looks good on 1.x branch? I had to make some tweaks

Kh4L

LGTM! with some nit

Kh4L · 2020-08-18T18:17:09Z

example/extensions/lib_custom_op/gemm_lib.cc

@@ -159,7 +161,7 @@ MXReturnValue inferShape(const std::unordered_map<std::string, std::string>& att
  unsigned kk = inshapes->at(1)[0];
  unsigned m = inshapes->at(1)[1];
  if (k != kk) {
-    std::cout << "Exected first input axis 1 equals to second input axis 0" << std::endl;
+    MX_ERROR_MSG << "Exected first input axis 1 equals to second input axis 0";


Suggested change

MX_ERROR_MSG << "Exected first input axis 1 equals to second input axis 0";

MX_ERROR_MSG << "Expected first input axis 1 equals to second input axis 0";

Kh4L · 2020-08-18T18:20:15Z

example/extensions/lib_custom_op/transposerowsp_lib.cc

-              << " Found output data type:" << outputs->at(0).dtype << std::endl;
+    MX_ERROR_MSG << "Error! Expected all inputs and outputs to be the same type."
+                 << "Found input storage type:" << inputs->at(0).stype
+                 << " Found output storage type:" << outputs->at(0).stype


Unwanted whitespaces?

LGTM! with some nit

Thanks! will add these todos to my next PR

example/extensions/lib_subgraph/subgraph_lib.cc

initial commit

6cb4dd3

samskalicky requested review from anirudh2290 and eric-haibin-lin as code owners August 10, 2020 20:26

samskalicky requested review from aaronmarkham and szha as code owners August 15, 2020 05:59

Ubuntu added 7 commits August 15, 2020 07:10

changed messages

f5abf26

changed to pointers and types

5c583c2

added cast

0a9302c

updated cast

db223ab

fixed signed int

a4b15f6

whitespace

9efad5a

Merge branch 'v1.x' of https://github.com/apache/incubator-mxnet into…

d623aba

… subgraph_v18

fixd pass resource

195232b

samskalicky mentioned this pull request Aug 17, 2020

[RFC] v1.8.0 release #18800

Open

Merge branch 'v1.x' of https://github.com/apache/incubator-mxnet into…

7e2a19c

… subgraph_v18

Kh4L approved these changes Aug 18, 2020

View reviewed changes

samskalicky mentioned this pull request Aug 18, 2020

MXNet external operators #18904

Closed

HahTK reviewed Aug 18, 2020

View reviewed changes

example/extensions/lib_subgraph/subgraph_lib.cc Show resolved Hide resolved

example/extensions/lib_subgraph/subgraph_lib.cc Show resolved Hide resolved

HahTK approved these changes Aug 18, 2020

View reviewed changes

samskalicky merged commit d1ac7c8 into apache:v1.x Aug 18, 2020

szha added this to the v1.8.0 milestone Aug 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1.x] Backporting #18779 to v1.x #18894

[1.x] Backporting #18779 to v1.x #18894

samskalicky commented Aug 10, 2020

mxnet-bot commented Aug 10, 2020

HahTK commented Aug 10, 2020

HahTK commented Aug 10, 2020

samskalicky commented Aug 15, 2020

samskalicky commented Aug 16, 2020

szha commented Aug 16, 2020

samskalicky commented Aug 16, 2020

samskalicky commented Aug 18, 2020

Kh4L left a comment

Kh4L Aug 18, 2020

Kh4L Aug 18, 2020

samskalicky Aug 18, 2020

	MX_ERROR_MSG << "Exected first input axis 1 equals to second input axis 0";
	MX_ERROR_MSG << "Expected first input axis 1 equals to second input axis 0";

[1.x] Backporting #18779 to v1.x #18894

[1.x] Backporting #18779 to v1.x #18894

Conversation

samskalicky commented Aug 10, 2020

Description

Checklist

Essentials

Changes

Comments

mxnet-bot commented Aug 10, 2020

HahTK commented Aug 10, 2020

HahTK commented Aug 10, 2020

samskalicky commented Aug 15, 2020

samskalicky commented Aug 16, 2020

szha commented Aug 16, 2020

samskalicky commented Aug 16, 2020

samskalicky commented Aug 18, 2020

Kh4L left a comment

Choose a reason for hiding this comment

Kh4L Aug 18, 2020

Choose a reason for hiding this comment

Kh4L Aug 18, 2020

Choose a reason for hiding this comment

samskalicky Aug 18, 2020

Choose a reason for hiding this comment