[Fix] Fix dist training infinite waiting issue

[fingertap]

Motivation

If the log_vars has different length, GPUs will wait infinitely. This PR provides a fix for this by raising an assertion error when different GPUs have different length of log_vars.

Related: #1034

Modification

Add a cross-GPU communication to determine whether the GPUs have the same log_var length. If not, raise an assertion error.

BC-breaking (Optional)

None.

Use cases (Optional)

None.

Checklist

• Pre-commit or other linting tools are used to fix the potential lint issues.
• The modification is covered by complete unit tests. If not, please add more unit test to ensure the correctness.
• If the modification has potential influence on downstream projects, this PR should be tested with downstream projects, like MMDet or MMDet3D.
• The documentation has been modified accordingly, like docstring or example tutorials.

[CLAassistant]

All committers have signed the CLA.

[Junjun2016]

Hi @fingertap
Thanks for your contribution.

[fingertap]

The unittest fails, but I think it is not due to this fix.

[Junjun2016]

Could you please provide more cases?
Since acc will also exist in the log_vars.

[fingertap]

What cases do you need? I encountered this issue when using a batch size = 1 and the length of log_vars in different GPUs differs. This will cause some GPU to leave the for-loop earlier than others, leading to the rest of GPUs waiting for responses forever.

[Junjun2016]

The unittest fails, but I think it is not due to this fix.

Will fix it in this PR #1037.

[Junjun2016]

What cases do you need? I encountered this issue when using a batch size = 1 and the length of log_vars in different GPUs differs. This will cause some GPU to leave the for-loop earlier than others, leading to the rest of GPUs waiting for responses forever.

Could you provide the missed keys in log_vars?

[fingertap]

What cases do you need? I encountered this issue when using a batch size = 1 and the length of log_vars in different GPUs differs. This will cause some GPU to leave the for-loop earlier than others, leading to the rest of GPUs waiting for responses forever.

Could you provide the missed keys in log_vars?

This is defined by user. For example, say I have a model that predicts segmentation mask for the image. The image has a boolean flag. We only calculate the loss when the flag is true. Then for images with a false flag, we do not need to calculate accuracy for it. In this case, I may implement something in my forward_train like this:

if flag:
    losses['acc'] = accuracy(pred, label)
return losses

When parsing this losses variable, the GPUs will hang. It is hard to find the cause when the code is complicated.

[fingertap]

For more details, I quote the explanation from #1030 :

In the _parse_log function of the mmseg.segmentors.base.BaseSegmentor, it attempts to synchronize the loss values among all GPUs. What happened is that in this loop (line 194):

for loss_name, loss_value in log_vars.items():
   # reduce loss when distributed training
   if dist.is_available() and dist.is_initialized():
       loss_value = loss_value.data.clone()
       dist.all_reduce(loss_value.div_(dist.get_world_size()))
   log_vars[loss_name] = loss_value.item()

One GPU A does not have a "roi_acc" as loss_name (suppose it is the last key in log_vars). Then this GPU A thinks it has done all its work, and jump out of the loop. Other GPUs with the last "roi_acc" will try to call torch.distributed.all_reduce, which infinitely waits for the reply from GPU A.

[Junjun2016]

What cases do you need? I encountered this issue when using a batch size = 1 and the length of log_vars in different GPUs differs. This will cause some GPU to leave the for-loop earlier than others, leading to the rest of GPUs waiting for responses forever.

Could you provide the missed keys in log_vars?

This is defined by user. For example, say I have a model that predicts segmentation mask for the image. The image has a boolean flag. We only calculate the loss when the flag is true. Then for images with a false flag, we do not need to calculate accuracy for it. In this case, I may implement something in my forward_train like this:

if flag:
    losses['acc'] = accuracy(pred, label)
return losses

When parsing this losses variable, the GPUs will hang. It is hard to find the cause when the code is complicated.

I see.
If this is a general case for bs=1 on a customized dataset, how can we avoid or fix the bug?

[fingertap]

We can inform the user about this issue, instead of letting the GPUs hang. I did this by triggering an assertion error:

# If the loss_vars has different length, GPUs will wait infinitely
if dist.is_available() and dist.is_initialized():
    log_var_length = torch.tensor(len(log_vars), device=loss.device)
    dist.all_reduce(log_var_length)
    assert log_var_length == len(log_vars) * dist.get_world_size(), \
        'loss log variables are different across GPUs!'

To allow the users skipping these loss terms, the users should explicitly set these loss terms as None. To illustrate this:

# Don't do this
if flag:
    losses['acc'] = accuracy(pred, label)

# Do this
losses['acc'] = accuracy(pred, label) if flag else None

It is the users' responsibility to ensure these losses have identical keys among GPUs to prevent synchronization issues. What we need to do is to allow such None values for loss_vars. The complete fix I came up with is (modifications are marked with # HERE):

@staticmethod
def _parse_losses(losses):
    """Parse the raw outputs (losses) of the network.

    Args:
        losses (dict): Raw output of the network, which usually contain
            losses and other necessary information.

    Returns:
        tuple[Tensor, dict]: (loss, log_vars), loss is the loss tensor
            which may be a weighted sum of all losses, log_vars contains
            all the variables to be sent to the logger.
    """
    log_vars = OrderedDict()
    for loss_name, loss_value in losses.items():
        if loss_value is None:  # HERE
            log_vars[loss_name] = None
        elif isinstance(loss_value, torch.Tensor):
            log_vars[loss_name] = loss_value.mean()
        elif isinstance(loss_value, list):
            log_vars[loss_name] = sum(_loss.mean() for _loss in loss_value)
        else:
            raise TypeError(
                f'{loss_name} is not a tensor or list of tensors')

    loss = sum(_value for _key, _value in log_vars.items()
               if 'loss' in _key)

    # HERE
    # If the loss_vars has different length, GPUs will wait infinitely
    if dist.is_available() and dist.is_initialized():
        log_var_length = torch.tensor(len(log_vars), device=loss.device)
        dist.all_reduce(log_var_length)
        assert log_var_length == len(log_vars) * dist.get_world_size(), \
            'loss log variables are different across GPUs!'

    log_vars['loss'] = loss
    for loss_name, loss_value in log_vars.items():
        # reduce loss when distributed training
        # HERE
        if dist.is_available() and dist.is_initialized():
            if loss_value is not None:
                sync_value = torch.tensor(
                    [loss_value.data.item(), 1.], device=loss.device)
            else:
                sync_value = torch.tensor([0., 0.], device=loss.device)
            dist.all_reduce(sync_value)
            # calculate mean if loss_value in some GPUs are not None
            if not sync_value[1] == 0.:
                log_vars[loss_name] = sync_value[0] / sync_value[1]

    log_vars = {k:v.item() for k, v in log_vars.items() if v is not None}

    return loss, log_vars

[fingertap]

However, in this PR I did not include my fix for the None log_vars, as I think this is kind of a new feature rather than a bug. This PR is about the bug that GPUs hang without any useful error trace. If you decide to include this fix into other PRs, I suggest to include those features for a None value, too.

[Junjun2016]

Hi @fingertap
Thanks for your nice suggestions, we will discuss more about it.
Please merge the master to fix the failed CI.

[Junjun2016]

@hhaAndroid
Could you please give some comments?

[fingertap]

Hi @fingertap Thanks for your nice suggestions, we will discuss more about it. Please merge the master to fix the failed CI.

The merging is blocked because of the test_mit testcase failed, which I think has nothing to do with my fix.

[hhaAndroid]

@hhaAndroid Could you please give some comments?

Done

[Junjun2016]

Has been fixed in the master, so you could merge the upstream master.<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> </head> <body> <br/><br/><br/> <div style="display: inline-block;"> <HR width=100% align=left size=0.5 color=#eaebec noshade> <font color='#78787A'> 发自 </font> <a href='https://mail.163.com/dashi/dlpro.html?from=mail81' target='_blank' style='text-decoration: none; color: #111112'>网易邮箱大师</a> </div> <br/><br/><br/><div class="ntes-mailmaster-quote" style="padding-top: 1px; padding-bottom: 1px" > <div style=" margin-top: 2em; margin-bottom: 1em; font-size: 14px; line-height: 1.25; color: #78787a; " >---- 回复的原邮件 ----</div> <div style=" margin-bottom: 1em; font-size: 12px; line-height: 1.25; color: #232324; padding: 0.5em 0.25em; border-radius: 0.67em; background-color: #f0f0f0; " > <table width="100%" cellpadding="0" cellspacing="9" border="0"> <tr> <td valign="top" style=" width: 4.25em; font-size: 12px; line-height: 1.25; color: #78787a; " > 发件人 </td> <td valign="top" style=" font-size: 12px; line-height: 1.25; color: #232324; word-break: break-all; " > <a class="mail-from" style="text-decoration:none;color:#0886e8;" ***@***.***">Han ***@***.***&gt;</a> </td> </tr> <tr> <td valign="top" style=" width: 4.25em; font-size: 12px; line-height: 1.25; color: #78787a; " > 日期 </td> <td class="mail-date" valign="top" style=" font-size: 12px; line-height: 1.25; color: #232324; word-break: break-all; " > 2021年11月15日 10:58 </td> </tr> <tr style=""> <td valign="top" style=" width: 4.25em; font-size: 12px; line-height: 1.25; color: #78787a; " > 收件人 </td> <td valign="top" style=" font-size: 12px; line-height: 1.25; color: #232324; word-break: break-all; " > <a class="mail-to" style="text-decoration:none;color:#0886e8;" ***@***.******@***.***&gt;</a> </td> </tr> <tr style=""> <td valign="top" style=" width: 4.25em; font-size: 12px; line-height: 1.25; color: #78787a; " > 抄送至 </td> <td valign="top" style=" font-size: 12px; line-height: 1.25; color: #232324; word-break: break-all; " > <a class="mail-cc" style="text-decoration:none;color:#0886e8;" ***@***.******@***.***&gt;</a>、<a class="mail-cc" style="text-decoration:none;color:#0886e8;" ***@***.******@***.***&gt;</a> </td> </tr> <tr> <td valign="top" style=" width: 4.25em; font-size: 12px; line-height: 1.25; color: #78787a; " > 主题 </td> <td class="mail-subject" valign="top" style=" font-size: 12px; line-height: 1.25; color: #232324; word-break: break-all; " > Re: [open-mmlab/mmsegmentation] [#1034] fix dist training infinite waiting issue (PR #1035) </td> </tr> </table> </div> <div><p></p> <blockquote> <p dir="auto">Hi <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/users/fingertap/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" ***@***.***</a> Thanks for your nice suggestions, we will discuss more about it. Please merge the master to fix the failed CI.</p> </blockquote> <p dir="auto">The merging is blocked because of the test_mit testcase failed, which I think has nothing to do with my fix.</p> <p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">&mdash;<br />You are receiving this because you commented.<br />Reply to this email directly, <a href="#1035 (comment)">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AD7REKJWZZNE6GJNDLEHOUTUMBZL3ANCNFSM5H6DJPBQ">unsubscribe</a>.<br />Triage notifications on the go with GitHub Mobile for <a href="https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&amp;mt=8&amp;pt=524675">iOS</a> or <a href="https://play.google.com/store/apps/details?id=com.github.android&amp;referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub">Android</a>. <img src="https://github.com/notifications/beacon/AD7REKIABDFMBXXMBI3LG7LUMBZL3A5CNFSM5H6DJPB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOHG46FLQ.gif" height="1" width="1" alt="" /></p> <script type="application/ld+json">[ { ***@***.***": "http://schema.org", ***@***.***": "EmailMessage", "potentialAction": { ***@***.***": "ViewAction", "target": "#1035 (comment)", "url": "#1035 (comment)", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { ***@***.***": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script></div> </div> </body> </html>

[Junjun2016]

Hi @fingertap Thanks for your nice suggestions, we will discuss more about it. Please merge the master to fix the failed CI.

The merging is blocked because of the test_mit testcase failed, which I think has nothing to do with my fix.

The BC-breaking has been fixed in the master branch, so you can merge the master branch to your branch.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Project coverage is 89.48%. Comparing base (6b3e550) to head (84858fc).
Report is 300 commits behind head on master.

Files	Patch %	Lines
mmseg/models/segmentors/base.py	0.00%	4 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1035      +/-   ##
==========================================
- Coverage   89.55%   89.48%   -0.07%     
==========================================
  Files         119      119              
  Lines        6626     6631       +5     
  Branches     1034     1035       +1     
==========================================
  Hits         5934     5934              
- Misses        488      492       +4     
- Partials      204      205       +1     

Flag	Coverage Δ
unittests	89.48% <0.00%> (-0.07%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[fingertap]

Fixed the linting issue and pulled the upstream master.

[xvjiarui]

Hi @fingertap
Sorry for the late reply. I will review it shortly.

[Junjun2016]

Hi @hhaAndroid
Could you please provide the link of the same PR in mmdet and review it again?

[hhaAndroid]

Hi @hhaAndroid Could you please provide the link of the same PR in mmdet and review it again?

open-mmlab/mmdetection#6501