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

[MXNET-1255] update hybridize documentation #13597

Merged
merged 5 commits into from
Jan 7, 2019

Conversation

roywei
Copy link
Member

@roywei roywei commented Dec 10, 2018

Description

This is the first step to address issues in:
#10875
#12100
#12109

First provide documentation and current work around.

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 http: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

Add documentations for operators does not work with hybridization

Comments

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

@roywei roywei requested a review from szha as a code owner December 10, 2018 04:57
@roywei
Copy link
Member Author

roywei commented Dec 12, 2018

@mxnet-label-bot add[Doc, pr-awaiting-review]

@marcoabreu marcoabreu added Doc pr-awaiting-review PR is waiting for code review labels Dec 12, 2018
Copy link
Contributor

@ChaiBapchya ChaiBapchya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty handy doc! Thanks for the summarization.

docs/tutorials/gluon/hybrid.md Outdated Show resolved Hide resolved
return x[0]
```

The current workaround is explicitly call [`slice`](https://mxnet.incubator.apache.org/api/python/ndarray/ndarray.html#mxnet.ndarray.NDArray.slice) operators in `hybrid_forwar`d.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit : hybrid_forward

```

### Slice
[] in NDArray is to get a slice from the array. [] in Symbol is to get an output from a grouped symbol.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit : how about
[] in NDArray is used to get a slice from the array. However, [] in Symbol is used to get an output from a grouped symbol.

return x[0]
```

The current workaround is explicitly call [`slice`](https://mxnet.incubator.apache.org/api/python/ndarray/ndarray.html#mxnet.ndarray.NDArray.slice) operators in `hybrid_forwar`d.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: to explicitly call


#### In-Place Arithmetic Operators

In place arithmetic operators are also used a lot in Gluon imperative mode. You need to avoid that and write the operations explicitly in `hybrid_forward`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit : In-place

#### In-Place Arithmetic Operators

In place arithmetic operators are also used a lot in Gluon imperative mode. You need to avoid that and write the operations explicitly in `hybrid_forward`.
For example, avoid `x += y` and use `x = x + y`, other wise you will get `NotImplementedError`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: otherwise

@roywei
Copy link
Member Author

roywei commented Dec 18, 2018

@aaronmarkham @zheng-da could you help review? Thanks!

Copy link
Contributor

@aaronmarkham aaronmarkham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made some suggested changes.

Line 3 mentions a newer version. That will deflect most traffic to this page, yet you're updating it with a lot of info. I'd Change that line to read:

## Related Content
* [Fast, portable neural networks with Gluon HybridBlocks](https://gluon.mxnet.io/chapter07_distributed-learning/hybridize.html)

Also, to clean up the formatting, why not try to use explicit math output with the math directive.

:math: `x.iadd(y)` <=> `x+=y` 

@@ -137,4 +137,94 @@ to gluon with `SymbolBlock`:
net2 = gluon.SymbolBlock.imports('model-symbol.json', ['data'], 'model-0001.params')
```

## Operators that does not work with hybridize
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
## Operators that does not work with hybridize
## Operators that do not work with hybridize

@@ -137,4 +137,94 @@ to gluon with `SymbolBlock`:
net2 = gluon.SymbolBlock.imports('model-symbol.json', ['data'], 'model-0001.params')
```

## Operators that does not work with hybridize

If you want to hybridize your model, you must use `F.some_operator` in your 'hybrid_forward' function, F will be 'mxnet.nd' before you hybridize and 'mxnet.sym' after hybridize. While most APIs are the same in NDArray and Symbol, there are some differences. Writing `F.some_operator` and call `hybridize` may not work all the time.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If you want to hybridize your model, you must use `F.some_operator` in your 'hybrid_forward' function, F will be 'mxnet.nd' before you hybridize and 'mxnet.sym' after hybridize. While most APIs are the same in NDArray and Symbol, there are some differences. Writing `F.some_operator` and call `hybridize` may not work all the time.
If you want to hybridize your model, you must use `F.some_operator` in your 'hybrid_forward' function.
`F` will be 'mxnet.nd' before you hybridize and 'mxnet.sym' after hybridize. While most APIs are the same in NDArray and Symbol, there are some differences. Writing `F.some_operator` and call `hybridize` may not work all of the time.

### Element-wise Operators

The following arithmetic and comparison APIs are automatically broadcasted if the input NDArrays have different shapes.
However, that's not the case in Symbol API, it's not automatically broadcasted and you have to manually specify whether to use element-wise operator or broadcast operators.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
However, that's not the case in Symbol API, it's not automatically broadcasted and you have to manually specify whether to use element-wise operator or broadcast operators.
However, that's not the case in Symbol API. It's not automatically broadcasted, and you have to manually specify whether to use element-wise operator or broadcast operators.

docs/tutorials/gluon/hybrid.md Show resolved Hide resolved
docs/tutorials/gluon/hybrid.md Show resolved Hide resolved

### Slice
`[]` in NDArray is used to get a slice from the array. However, `[]` in Symbol is used to get an output from a grouped symbol.
For example, you will get different result for the following method before and after hybridization.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
For example, you will get different result for the following method before and after hybridization.
For example, you will get different results for the following method before and after hybridization.


### Not implemented operators

Some of the often used operators in NDArray are not implemented in Symbol, and will cause hybridization failure
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Some of the often used operators in NDArray are not implemented in Symbol, and will cause hybridization failure
Some of the often used operators in NDArray are not implemented in Symbol, and will cause hybridization failure.


#### Array API

`mx.nd.array()` is used a lot but Symbol does not have the array API. The current workaround is to use F.ones/ F.zeros/ F.full which exists in both NDArrays and Symbols.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`mx.nd.array()` is used a lot but Symbol does not have the array API. The current workaround is to use F.ones/ F.zeros/ F.full which exists in both NDArrays and Symbols.
`mx.nd.array()` is used a lot, but Symbol does not have the `array` API. The current workaround is to use `F.ones`, `F.zeros`, or `F.full`, which exist in both the NDArray and Symbol APIs.


#### In-Place Arithmetic Operators

In-place arithmetic operators are also used a lot in Gluon imperative mode. You need to avoid that and write the operations explicitly in `hybrid_forward`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"avoid that"? What exactly? All in-place operators? Can you clarify this?
Maybe better to say something like...

Suggested change
In-place arithmetic operators are also used a lot in Gluon imperative mode. You need to avoid that and write the operations explicitly in `hybrid_forward`.
In-place arithmetic operators may be used in Gluon imperative mode, however if you expect to hybridize, you should write the operations explicitly instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avoid all in-place operators. I have added example: avoid x+=y and use x = x + y.


| NDArray in-place arithmetic operators | Description |
|---|---|
|[*NDArray.\__iadd\__*](https://mxnet.incubator.apache.org/api/python/ndarray/ndarray.html#mxnet.ndarray.NDArray.__iadd__) | x.__iadd__(y) <=> x+=y |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice that the double underscore is bolding the text. I'd be worried what this will be interpreted as by Sphinx/rst down the line.
You can escape it, or like I mentioned before use :math:.

@roywei
Copy link
Member Author

roywei commented Dec 19, 2018

@aaronmarkham @ChaiBapchya Thanks for the review, I have addressed your comments.

  1. I have also added dive into deeplearning book as related content, let me know if it's not ready.
  2. For the table format, I was following the format on website. As for the math formulas, it seems only renders on website, but not on downloaded jupyter notebooks. Let me knwo if you have other suggestions.
  3. Added asnumpy and a summary section
    Thanks!

@roywei
Copy link
Member Author

roywei commented Dec 21, 2018

@ThomasDelteil @nswamy Could you help take a look? Thanks!

@@ -1,6 +1,9 @@
# Hybrid - Faster training and easy deployment

*Note: a newer version is available [here](http:https://gluon.mxnet.io/chapter07_distributed-learning/hybridize.html).*
*Related Contents:*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Content instead of Contents (but don't restart CI just for that change).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a link to the new dive into deeplearning book so made it "contents"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a semantic thing I guess. There's probably a rule here but... usually you can say Content is plural already (as it refers to a mass of stuff) and you don't usually say "Related Contents". It's more common to say "Related Content" even if there's more than one item in the list.
It is weird because you do say "Table of Contents", not "Table of Content".
🤷‍♂️ English is weird. It's not a showstopper... it's fine whichever way you want to do it. (but do it my way). Jk

Copy link
Contributor

@aaronmarkham aaronmarkham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super minor nits.


### Not implemented operators

Some of the often used operators in NDArray are not implemented in Symbol, and will cause hybridization failure
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Some of the often used operators in NDArray are not implemented in Symbol, and will cause hybridization failure
Some of the often used operators in NDArray are not implemented in Symbol, and will cause hybridization failure.

Some of the often used operators in NDArray are not implemented in Symbol, and will cause hybridization failure

#### NDArray.asnumpy
Symbol does not support `asnumpy` function, you need to avoid calling `asnumpy` in `hybrid_forward`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Symbol does not support `asnumpy` function, you need to avoid calling `asnumpy` in `hybrid_forward`.
Symbol does not support the `asnumpy` function. You need to avoid calling `asnumpy` in `hybrid_forward`.

@@ -1,6 +1,9 @@
# Hybrid - Faster training and easy deployment

*Note: a newer version is available [here](http:https://gluon.mxnet.io/chapter07_distributed-learning/hybridize.html).*
*Related Contents:*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
*Related Contents:*
*Related Content:*

@Roshrini
Copy link
Member

Roshrini commented Jan 2, 2019

Seems like all the comments are addressed.
@sandeep-krishnamurthy Can you merge this PR?
@mxnet-label-bot Update [pr-awaiting-merge]

@marcoabreu marcoabreu added pr-awaiting-merge Review and CI is complete. Ready to Merge and removed Doc pr-awaiting-review PR is waiting for code review labels Jan 2, 2019
@aaronmarkham aaronmarkham merged commit c63ef9a into apache:master Jan 7, 2019
rondogency pushed a commit to rondogency/incubator-mxnet that referenced this pull request Jan 9, 2019
* update hybridize documentation

* address review comments

* improve doc

* address comments

* address comments
haohuanw pushed a commit to haohuanw/incubator-mxnet that referenced this pull request Jun 23, 2019
* update hybridize documentation

* address review comments

* improve doc

* address comments

* address comments
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr-awaiting-merge Review and CI is complete. Ready to Merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants