This repository has been archived by the owner on Nov 17, 2023. It is now read-only.
Mitigate exponential complexity when running gpu_topology tests #13343
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@mxnet-label-bot add [pr-awaiting-review] |
larroy
force-pushed
the
gpu_topo
branch
2 times, most recently
from
4900f7c
to
9f6b9cd
Compare
|
LGTM, changing the test coverage from 16 to 8 should allow it finish much quicker. |
lebeg
reviewed
Nov 21, 2018
| @@ -67,15 +63,18 @@ bool IsSatisfactory(const std::vector<float>& W, int num_gpus, int depth) { | |||
| // Generates random link topology matrix using random number generator | |||
| void TestComputeTreesRandomized(int num_gpus, float alpha, int backtrack, | |||
| std::mt19937* gen) { | |||
| using namespace std; | |||
There was a problem hiding this comment.
You don't need it if you are not using it
| std::fill(W.begin(), W.end(), 0.f); | ||
| GenerateMatrix(&W, num_gpus, k, gen); | ||
| satisfied = IsSatisfactory(W, num_gpus, depth); | ||
| std::fill(W.begin(), W.end(), 0.f); |
There was a problem hiding this comment.
According to std::vector's constructor, the initialization is already done at line 69.
| @@ -562,7 +561,7 @@ TEST(GpuTopology, TestComputeTrees1) { | |||
| float alpha = 0.7; | |||
| bool backtrack = true; | |||
| // Do 5 randomized tests per GPU count from 2 to 16 | |||
There was a problem hiding this comment.
Change the comment from 16 to 8 as well
- KL never succeeds so it always goes exponential - Too many weight matrices were rejected because of zero weights, simplify generation to not include 0 weight edges
lebeg
reviewed
Nov 27, 2018
| satisfied = IsSatisfactory(W, num_gpus, depth); | ||
| GenerateMatrix(&W, num_gpus, gen); | ||
| satisfied = IsSatisfactory(W, num_gpus, depth); | ||
| if (mxnet::kvstore::kLogTree && !satisfied) { |
There was a problem hiding this comment.
Previously it was trying multiple times - we want to stop doing it with this change?
There was a problem hiding this comment.
yes, it's of no use. I also changed the distribution, is not relevant anymore and took a long time.
lebeg
approved these changes
Nov 29, 2018
aaronmarkham
pushed a commit
to aaronmarkham/incubator-mxnet
that referenced
this pull request
Nov 30, 2018
- KL never succeeds so it always goes exponential - Too many weight matrices were rejected because of zero weights, simplify generation to not include 0 weight edges
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Mitigates MXNET_KVSTORE_USETREE problems #13341
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.