Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed issue with memory format inconsistency for interpolate op #100373

Closed

Conversation

vfdev-5
Copy link
Collaborator

@vfdev-5 vfdev-5 commented May 1, 2023

Related to #68430

Description:

  • Output's memory format is not respected in interpolate op when input is squeezed/unsqueezed channels last

For example:

import torch
print(torch.__version__)


def t1(x, mode, aa):
    do_squeeze = False
    if x.dim() == 3:
        do_squeeze = True
        x = x[None, ...]
    out = torch.nn.functional.interpolate(
        x, size=224, mode=mode, antialias=aa
    )
    if do_squeeze:
        out = out[0, ...]
    return out


aa = True
mode = "bilinear"
dtype = torch.uint8
mf = torch.channels_last
device = "cpu"

print("-", aa, mode, mf, device, dtype)
x = torch.randint(0, 256, size=(1, 3, 256, 256), dtype=dtype, device=device).contiguous(memory_format=mf)

x = x[0, ...]
x = x[None, ...]

y = t1(x, mode, aa)

input_mem_format = "CL" if x.is_contiguous(memory_format=torch.channels_last) else "CF"
if input_mem_format == "CF":
    assert x.is_contiguous(memory_format=torch.contiguous_format)

output_mem_format = "CL" if y.is_contiguous(memory_format=torch.channels_last) else "CF"
if output_mem_format == "CF":
    assert y.is_contiguous(memory_format=torch.contiguous_format)

if input_mem_format != output_mem_format:
    print(f"2 {mf}, {device}, {dtype}: {output_mem_format} != {input_mem_format}\n")

We can see have the following output on pytorch nightly:

2 torch.channels_last, cpu, torch.uint8: CF != CL

In details, memory format inconsistency comes from the strides:

x = torch.ones(1, 3, 256, 256).contiguous(memory_format=torch.channels_last)
x = x[0, ...].unsqueeze(0)
assert x.stride() == (3, 1, 768, 3)  # instead of (196608, 1, 768, 3)

The problem essentially comes from x.unsqueeze(0) where new stride (i.e. stride(0)) is computed as for a tensor with channels first memory format:

int64_t new_stride = dim >= tensor.dim() ? 1 : result.sizes[dim] * result.strides[dim];

@vfdev-5 vfdev-5 force-pushed the fix-interp-output-memformat-1CHW-CL branch from cf459b8 to e438650 Compare May 3, 2023 12:16
@vfdev-5 vfdev-5 force-pushed the fix-interp-output-memformat-1CHW-CL branch from e438650 to f327ca8 Compare May 3, 2023 12:19
Comment on lines -2157 to -2182
# following "heuristic: only use channels_last path when it's faster than the contiguous path"
_, n_channels, _, _ = input.shape
if input.device.type == "cuda" and n_channels < 4:
memory_format = torch.contiguous_format
Copy link
Collaborator Author

@vfdev-5 vfdev-5 May 3, 2023

Choose a reason for hiding this comment

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

Removed this heuristics due to failing test_fake_tensor.py tests and also https://github.com/pytorch/pytorch/pull/91260/files#r1183144329
cc @jbschlosser

@vfdev-5 vfdev-5 marked this pull request as ready for review May 4, 2023 09:54
@ezyang ezyang requested a review from jbschlosser May 9, 2023 20:40
@ezyang ezyang added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label May 9, 2023
@ezyang
Copy link
Contributor

ezyang commented May 9, 2023

@jbschlosser holler if you are not the right person

@github-actions
Copy link

github-actions bot commented Jul 8, 2023

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

@github-actions github-actions bot added the Stale label Jul 8, 2023
@ezyang
Copy link
Contributor

ezyang commented Jul 9, 2023

@pytorchbot merge -r

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jul 9, 2023
@github-actions
Copy link

github-actions bot commented Jul 9, 2023

This PR needs a release notes: label

If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

Description:
- Output's memory format is not respected in interpolate op when input is squeezed/unsqueezed channels last
@pytorchmergebot
Copy link
Collaborator

Successfully rebased fix-interp-output-memformat-1CHW-CL onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout fix-interp-output-memformat-1CHW-CL && git pull --rebase)

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: This PR needs a release notes: label
If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Details for Dev Infra team Raised by workflow job

@vfdev-5 vfdev-5 marked this pull request as draft July 10, 2023 08:36
@vfdev-5
Copy link
Collaborator Author

vfdev-5 commented Jul 10, 2023

@ezyang thanks for the review and the approval. Right now I set this PR to draft as there are failing tests and we also plan to make more tests from torchvision-side

@github-actions github-actions bot closed this Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request open source Stale triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants