[MXNET-1294] Priority-based parameter propagation for improved data parallel training throughput #15124

anandj91 · 2019-06-01T21:03:24Z

Description

https://issues.apache.org/jira/browse/MXNET-1294

During data parallel training , the parameters of each layer is synchronized through parameter server after slicing them into smaller slices and synchronizing them independent to each other based on their priority. Currently priority of a layer is set as its negative index.

For more details, refer https://anandj.in/wp-content/uploads/sysml.pdf.

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

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

piyushghai · 2019-06-04T20:03:41Z

Thanks for your contribution @anandj91. Can you look into the CI failures ?
@mxnet-label-bot Add [pr-awaiting-review, Backend].

anandj91 · 2019-06-06T16:20:46Z

Is there a way to rerun the test cases without doing a git push?

piyushghai · 2019-06-06T16:43:31Z

Is there a way to rerun the test cases without doing a git push?

You could push an empty commit.
git commit -m "Empty commit" --allow-empty

That should trigger the CI on your PR again.

src/kvstore/kvstore_dist.h

vandanavk · 2019-06-16T19:17:02Z

@pinaraws @anirudhacharya for review

anandj91 · 2019-06-20T00:35:44Z

Modified the code to address the review comments. Sorry for the delay.
I have added a new flag in the config.mk to enable priority based update (USE_PRIORITY_UPDATE). the flag is disabled by default. @eric-haibin-lin Can you please tell me on how to add a new test case to build the code with this flag turned on and run the dist_kvstore test cases?

eric-haibin-lin

@anandj91 thanks for the update and sorry for the late reply.

Could you make it a runtime option with environment variable instead of compilation time flag? That makes it easier to distribute the feature (usually people just install mxnet with pip install).
Could you also update the env var page: https://github.com/apache/incubator-mxnet/blob/master/docs/faq/env_var.md
Did you do any perf testing after changing the implementation with NewVar()s?
For testing, you can add test cases in https://github.com/apache/incubator-mxnet/tree/master/tests/nightly and update on CI here: https://github.com/apache/incubator-mxnet/blob/7fe478afc4f8bed58b9816ddce57a3c75997c9f2/ci/docker/runtime_functions.sh#L1148

Thanks!

roywei · 2019-07-08T16:45:33Z

@anandj91 gentle ping, any update on this PR?

anandj91 · 2019-07-08T17:34:48Z

@roywei The current implementation uses multiple ThreadVar to specify dependency between push and pull between slices. After some benchmarking on large models like VGG-19, I found that this causes large overhead and the training performance reduce to 50%.

Instead I'm planning to introduce a new API for pushpull which combines push and pull of one slice. I had an offline discussion with @eric-haibin-lin and he is fine with this approach.

anandj91 · 2019-07-16T18:24:07Z

This PR is waiting on #15559 for the PushPull API in KVStore.

anandj91 · 2019-10-15T23:02:38Z

I'm facing some design level challenges to properly implement Priority based update (P3) on top of PushPull API. MXNet does a simple load balancing before pushing or pulling key-values by splitting NDArrays equally to the parameter servers. P3 requires a round-robin style parameter distribution which means slicing a large NDArray into thousands of smaller ones. Much more granular than current default distribution strategy and each PS would get more than one slice.

With the way mxnet and ps-lite designed right now, ps-lite assumes a single ZPush/ZPull/ZPushPull belongs to a single layer/NDArray. It also assumes that one slice only belong to one PS. These assumption need to be broken for implementing P3. What I have done right now is to add round-robin (RR) distribution strategy along with the default one and use a boolean flag to switch between these two. When user chooses to use RR, KVStore consider each slice as separate key-value pair. Otherwise fallback to the default mode.

eric-haibin-lin

A few comments on readability.

What would be the user experience for an end-user if he is interested in P3?

.gitmodules

src/kvstore/kvstore_dist.h

docs/static_site/src/pages/api/faq/env_var.md

src/kvstore/kvstore_dist.h

docs/static_site/src/pages/api/faq/env_var.md

python/mxnet/kvstore/base.py

python/mxnet/kvstore/kvstore.py

src/kvstore/p3store.h

include/mxnet/c_api.h

python/mxnet/kvstore/base.py

python/mxnet/kvstore/kvstore.py

eric-haibin-lin · 2020-01-25T08:39:19Z

python/mxnet/kvstore/kvstore.py

@@ -67,6 +68,7 @@ def __init__(self, handle):
        self._updater = None
        self._updater_func = None
        self._str_updater_func = None
+        self._is_p3 = (os.getenv('DMLC_PS_VAN_TYPE', '') == 'p3')


will u also change launch.py?

sure. I just wanted to confirm if we are planning to only add --p3 to launch.py now and not --horovod or --byteps because of the reasons I mentioned in the previous comment

Could you clarify a bit more on this? According to this RFC (#16795), p3 will be treated as another kvstore just like byteps/horovod. Why would it be different?

@anandj91 and I had a recent discussion. Having separate python kvstore class for p3 leads to lots of duplicated code because it also supports both device and local mode. Its source code lives inside mxnet, and sharing the same python KVStore class is reasonable in terms of complexity and maintenance efforts

@apeforest We do have a separate class for P3 in the backend though.

python/mxnet/model.py

src/kvstore/kvstore_dist.h

eric-haibin-lin · 2020-01-27T02:52:57Z

python/mxnet/kvstore/kvstore.py

@@ -349,6 +363,7 @@ def pushpull(self, key, value, out=None, priority=0):

        out: NDArray or list of NDArray or list of list of NDArray
            Values corresponding to the keys.
+            `out` should have length as `value`.


Why is there such a restriction?

there is grouping of keys and corresponding ndarrays happening at the backend based on their positions in the list before performing kv operations. Since we are using same key list for both value and out, their layouts should be identical. length check is just for a sanity.

this is the reason why I had both vkeys and okeys separate in the previous implementation.

You're right. However, if there's one key, having value = a list of 4 arrays, and out = a list of one array is also valid.

in that case we need to bring back separate key list for value and out.

python/mxnet/model.py

P3StoreDist extends KVStoreDist class to perform Push, Pull and PushPull based on the priority of the packets * Use --p3 flag in launch.py script to use P3 * DMLC_PS_VAN_TYPE - to turn on P3 with distributed kvstore * DMLC_PS_WATER_MARK - to set the maximum in-flight packets in the network queue Note: Currently P3 does not support: 1. operations on sparse ndarrays 2. gradient compression 3. update on kvstore server

eric-haibin-lin · 2020-02-02T00:05:26Z

@anandj91 great work!

P3StoreDist extends KVStoreDist class to perform Push, Pull and PushPull based on the priority of the packets * Use --p3 flag in launch.py script to use P3 * DMLC_PS_VAN_TYPE - to turn on P3 with distributed kvstore * DMLC_PS_WATER_MARK - to set the maximum in-flight packets in the network queue Note: Currently P3 does not support: 1. operations on sparse ndarrays 2. gradient compression 3. update on kvstore server

anandj91 requested a review from szha as a code owner June 1, 2019 21:03

szha requested a review from eric-haibin-lin June 1, 2019 21:08

eric-haibin-lin self-assigned this Jun 1, 2019

anandj91 force-pushed the prio branch from f20fb64 to f2dd8cb Compare June 1, 2019 22:10

anandj91 mentioned this pull request Jun 3, 2019

Priority-based Push and Pull dmlc/ps-lite#147

Merged

anandj91 force-pushed the prio branch from f2dd8cb to 803ced4 Compare June 3, 2019 20:00

marcoabreu added Backend Issues related to the backend of MXNet pr-awaiting-review PR is waiting for code review labels Jun 4, 2019

anandj91 force-pushed the prio branch 5 times, most recently from 07f9818 to d85967f Compare June 6, 2019 03:21

eric-haibin-lin reviewed Jun 7, 2019

View reviewed changes

anandj91 force-pushed the prio branch 2 times, most recently from 7ef001a to c64be49 Compare June 19, 2019 23:12

eric-haibin-lin reviewed Jun 26, 2019

View reviewed changes

anandj91 mentioned this pull request Jul 16, 2019

[MXNET-1294] Add PushPull API #15559

Merged

6 tasks

anandj91 force-pushed the prio branch from c64be49 to ebb4317 Compare October 15, 2019 22:05

anandj91 requested review from aaronmarkham and marcoabreu as code owners October 15, 2019 22:05

eric-haibin-lin reviewed Oct 16, 2019

View reviewed changes

anandj91 force-pushed the prio branch from ebb4317 to 439064b Compare January 17, 2020 09:55

anandj91 requested a review from anirudh2290 as a code owner January 17, 2020 09:55

anandj91 force-pushed the prio branch from b9a7efc to 2ce8589 Compare January 21, 2020 01:55

eric-haibin-lin reviewed Jan 21, 2020

View reviewed changes

eric-haibin-lin reviewed Jan 25, 2020

View reviewed changes

eric-haibin-lin reviewed Jan 26, 2020

View reviewed changes

python/mxnet/model.py Show resolved Hide resolved

src/kvstore/kvstore_dist.h Outdated Show resolved Hide resolved

eric-haibin-lin reviewed Jan 27, 2020

View reviewed changes

anandj91 force-pushed the prio branch 3 times, most recently from 0358357 to 876744b Compare January 31, 2020 17:37

anandj91 force-pushed the prio branch from 876744b to 62dcfa4 Compare January 31, 2020 22:30

eric-haibin-lin merged commit 65aab9e into apache:master Feb 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MXNET-1294] Priority-based parameter propagation for improved data parallel training throughput #15124

[MXNET-1294] Priority-based parameter propagation for improved data parallel training throughput #15124

anandj91 commented Jun 1, 2019 •

edited

Loading

piyushghai commented Jun 4, 2019

anandj91 commented Jun 6, 2019

piyushghai commented Jun 6, 2019 •

edited

Loading

vandanavk commented Jun 16, 2019

anandj91 commented Jun 20, 2019

eric-haibin-lin left a comment

roywei commented Jul 8, 2019

anandj91 commented Jul 8, 2019

anandj91 commented Jul 16, 2019

anandj91 commented Oct 15, 2019

eric-haibin-lin left a comment •

edited

Loading

eric-haibin-lin Jan 25, 2020

anandj91 Jan 25, 2020

apeforest Jan 30, 2020

eric-haibin-lin Jan 31, 2020 •

edited

Loading

anandj91 Jan 31, 2020

eric-haibin-lin Jan 27, 2020

anandj91 Jan 27, 2020

eric-haibin-lin Jan 27, 2020

anandj91 Jan 27, 2020

eric-haibin-lin commented Feb 2, 2020

[MXNET-1294] Priority-based parameter propagation for improved data parallel training throughput #15124

[MXNET-1294] Priority-based parameter propagation for improved data parallel training throughput #15124

Conversation

anandj91 commented Jun 1, 2019 • edited Loading

Description

Checklist

Essentials

Changes

Comments

piyushghai commented Jun 4, 2019

anandj91 commented Jun 6, 2019

piyushghai commented Jun 6, 2019 • edited Loading

vandanavk commented Jun 16, 2019

anandj91 commented Jun 20, 2019

eric-haibin-lin left a comment

Choose a reason for hiding this comment

roywei commented Jul 8, 2019

anandj91 commented Jul 8, 2019

anandj91 commented Jul 16, 2019

anandj91 commented Oct 15, 2019

eric-haibin-lin left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eric-haibin-lin Jan 31, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eric-haibin-lin commented Feb 2, 2020

anandj91 commented Jun 1, 2019 •

edited

Loading

piyushghai commented Jun 6, 2019 •

edited

Loading

eric-haibin-lin left a comment •

edited

Loading

eric-haibin-lin Jan 31, 2020 •

edited

Loading