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.

Sign up for GitHub

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

Jump to bottom

[fix] check for histogramdd when bins is int[] #100624

Closed
wants to merge 1 commit into from
Closed

[fix] check for histogramdd when bins is int[] #100624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions aten/src/ATen/native/Histogram.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,10 @@ std::vector<Tensor>& histogramdd_bin_edges_out_cpu(const Tensor& self, IntArrayR

auto outer_bin_edges = select_outer_bin_edges(reshaped_self, range);

const int64_t bin_size = bin_ct.size();
TORCH_CHECK(
N == bin_size,
"histogramdd: The size of bins must be equal to the innermost dimension of the input.");
for (const auto dim : c10::irange(N)) {
linspace_out(outer_bin_edges.first[dim], outer_bin_edges.second[dim],
bin_ct[dim] + 1, bin_edges_out[dim]);
Copy link
Collaborator Author

@kshitij12345 kshitij12345 May 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We'd end up reading garbage value for bin_ct[dim] for dim > bin_size.

import torch

# invalid bins
bins = [1, 1, 1, 1, 1]

# Valid bins, all asserts pass.
# bins = [1, 1, 1, 1, 1, 1]

def fn(input):
    return torch.histogramdd(input, bins)

x = torch.rand([5, 6], dtype=torch.float32)

# For invalid bins, consecutive call return incorrect output (with different shapes)
o1 = fn(x)
o2 = fn(x)
for o_, oo_ in zip(o1, o2):
    # AssertionError: The values for attribute 'shape' do not match: torch.Size([1, 1, 1, 1, 1, 273]) != torch.Size([1, 1, 1, 1, 1, 33]).
    torch.testing.assert_close(o_, oo_)

Expand Down
9 changes: 9 additions & 0 deletions torch/testing/_internal/common_methods_invocations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2748,6 +2748,12 @@ def sample_inputs_histogramdd(op_info, device, dtype, requires_grad, **kwargs):
yield SampleInput(input_tensor, bins_tensor,
weight=weight_tensor, density=density)

def error_inputs_histogramdd(opinfo, device, **kwargs):
invalid_bins = [1, 1, 1, 1, 1]
make_arg = partial(make_tensor, dtype=torch.float, device=device, requires_grad=False)
msg = "histogramdd: The size of bins must be equal to the innermost dimension of the input."
yield ErrorInput(SampleInput(make_arg(5, 6), invalid_bins), error_regex=msg)

def sample_inputs_histc(op_info, device, dtype, requires_grad, **kwargs):
make_arg = partial(make_tensor, dtype=dtype, device=device, requires_grad=requires_grad)

Expand Down Expand Up @@ -16024,8 +16030,11 @@ def reference_flatten(input, start_dim=0, end_dim=-1):
dtypes=floating_types(),
dtypesIfCUDA=_dispatch_dtypes(), # histogramdd is only implemented on CPU
sample_inputs_func=sample_inputs_histogramdd,
error_inputs_func=error_inputs_histogramdd,
supports_autograd=False,
skips=(
# Not implemented on CUDA
DecorateInfo(unittest.expectedFailure, 'TestCommon', 'test_errors', device_type='cuda'),
Comment on lines +16036 to +16037
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's add it for the cuda path now that we're at it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This operator is not implemented for CUDA.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's a bug in the implementation of test_errors then I guess. That's fine.

DecorateInfo(unittest.expectedFailure, 'TestNormalizeOperators', 'test_normalize_operator_exhaustive'),
# JIT tests don't work with Tensor keyword arguments
# https://github.com/pytorch/pytorch/issues/58507
Expand Down
Loading