Skip to content

Conversation

@teng-li
Copy link
Contributor

@teng-li teng-li commented Nov 6, 2018

We only need this for backward, for FWD cast, the non-fine-grained bucketing should be better since it's sequential anyway.

Test should be covered all by c10d test, reduced bucket size to make bucketing happen in c10d test.

@teng-li teng-li added the oncall: distributed Add this issue/PR to distributed oncall triage queue label Nov 6, 2018
@teng-li teng-li closed this Nov 6, 2018
@teng-li teng-li reopened this Nov 6, 2018
@teng-li teng-li closed this Nov 6, 2018
@teng-li teng-li reopened this Nov 6, 2018
@teng-li
Copy link
Contributor Author

teng-li commented Nov 6, 2018

@pytorchbot retest this please

Copy link
Contributor

@pietern pietern left a comment

Choose a reason for hiding this comment

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

Nice. One reserve call missing. Did you do a perf comparison for this? Curious to hear what happens.

std::vector<std::vector<at::Tensor>> bucketedTensors;
auto tensorGroups =
torch::utils::take_tensors(tensors, bucketSize, fineGrained);
for (auto& tensorGroup : tensorGroups) {

This comment was marked as off-topic.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@teng-li is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@teng-li is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@teng-li teng-li changed the title [c10d] Let DDP to use finer bucketing strategy for fp16 perf improvement [c10d] Added the finer bucketing option for DDP Nov 6, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@teng-li has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

oncall: distributed Add this issue/PR to distributed oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants