🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/125262

• 📄 Preview Python docs built from this PR
• 📄 Preview C++ docs built from this PR
• ❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit e919719 with merge base 52c7c89 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

Most likely the right fix indeed.
cc @cpuhrsch what is the right way to test these PRs?

Hi @nowtryz - Thanks for sending the PR! The tests are in https://github.com/pytorch/pytorch/blob/main/test/test_maskedtensor.py .

@nowtryz - You should be able to run this with pytest test/test_maskedtensor.py. You'll need to extend the tests to also cover fold/unfold. For example see [test_prod](

Hi,

Ok perfect, I confused test_masked.py and test_maskedtensor.py.
I added the test and updated the documentation.

From what I see fold is only implemented through torch.nn.Fold/torch.nn.functional.fold and would need to add a passthrough for im2col (and col2im for symmetry) but I don't really know how to test it from the MaskedTensor perspective, adding a test in test_maskedtensor with a torch.nn.Fold?

@nowtryz - Yes, exactly. A test_fold method for example.

Hi,

From what I understand, torch.nn.Fold has a specific semantic that requires a custom masked implementation: what should happen when I fold a combination of specified and unspecified values? Should I sum only the specified values or consider the fold unspecified? Consequently, I feel that this is out of the perimeter of this pull request. Feel free to correct me if I'm wrong.

In the meantime, I added support for torch.nn.Unfold/torch.nn.functional.unfold.

@nowtryz - Hm, on reduction semantics see details here: https://pytorch.org/tutorials/prototype/maskedtensor_advanced_semantics#reduction-semantics and also pytorch/rfcs#27 .

One way to derive semantics of composites such as Fold (or matmul) is by inferring them from the semantics of its components.

Or said different (and maybe less confusingly), if you were to implement Fold in terms of existing MaskedTensor operations (if that's possible), what would it do?

These kind of discussions are really important (in my very biased opinion) to separate out all the subtly different use cases: raggedness (tensor[i].size() and tensor[j].size() may not match for i != j), sparsity (storage layouts for Tensors with lots of zeros) and masking (logical elements that have a position, but no value).

So, for example, if you mask out an element it doesn't have a value, but it does still have a position. Should that influence the denominator in variance? If you do sparsity, well, 0 is still a value. So if you do softmax, you want to include that (but often people actually would rather it mean -inf because really they want to mask it out). But then if you do raggedness, well you just want to actually describe an entirely different shape. You want to entirely remove elements (e.g. sentences of different lengths) and not have their individual shape influence operations that are truly batched (e.g. parallel or commute with vmap along dimension 0).

Hi @cpuhrsch,

I'm trying to catch with the Fold/Unfold operators as they are different from Tensor.unfold. Looking at the operator's documentation, I understand the masked semantics of this operator like this:

If we have blocks of size $L$, then the folded cell would have the value $L \times x$, $x$ being the initial value of an equivalent Unfold operation yielding the given block. In that sens, if we have $N$ specified elements, $N \le L$, my understanding is that the reduction of the given block should be

$$\frac{L}{N} \sum_{n=1}^{N} x_n $$

If I am correct, then I am not sure how to implement if efficiently. My first idea (to reuse the existing ATen operator without further modifications) is the following:

1. the new mask is computed by passing the original one through the operator, which yields True for each block where at least one value was specified;
2. we compute the normal folded version of the data after filling unspecified values with 0s, let's call it "folded data";
3. the original mask is converted to an integer type and passed through the operator, which yields the $N$ values mentioned earlier, let's call it "divisors";
4. the result would be res = input.size(-1) * divisors * folded_data.

I don't know if it is more optimized, but I can also combine (1) and (3) by performing (3) and converting the result back to boolean to obtain the new mask.

What do you think?

Hi @cpuhrsch @mruberry,

Could we merge this PR and I will open a new one for the Fold when I have more time?

Many thanks

@pytorchbot rebase

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

Successfully rebased patch onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout patch && git pull --rebase)

@nowtryz I had to unexpectedly go on leave due to family reasons, but let's revisit this. Are there tests for im2col or col2im that you could add the additional dtype to? Otherwise this looks fine, thank you for sending this :)

Hi @cpuhrsch,

No problem! If I understand correctly, updating torch/testing/_internal/common_methods_invocations.py should have generated the correct tests for im2col and col2im. Though the tests fail for these functions on cuda with the dtype I added. I have to check the cuda kernel but the fix should be easy (however I do not know cuda)

@nowtryz - Thanks for double checking, please ping again if you need more pointers.

@pytorchbot rebase

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

Successfully rebased patch onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout patch && git pull --rebase)

Hi @cpuhrsch!
Last failure didn't seem to be related to the code of this PR and passed locally.

@pytorchbot merge

This PR needs to be approved by an authorized maintainer before merge.

@pytorchbot merge

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

I believe this caused a memory leak in test_maskedtensor.py::TestBasicsCUDA::test_stack_cuda GH job link HUD commit link, @nowtryz could you take a look?

@clee2000 - I think we'll need to revert this again then.

@clee2000 That's strange, the test is just doing a passthrough to the stack function, there is no change in the cuda kernels as part of this test, I will need to take a deeper look