🔗 Helpful links

• 🧪  See artifacts and rendered test results at hud.pytorch.org/pr/79526
• 📄  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 our office hours

✅ No Failures (0 Pending)

As of commit 5fcaf70 (more details on the Dr. CI page):

I don't think we need these to be prims, do we, @Aidyn-A?

I don't think we need these to be prims, do we, @Aidyn-A?

Hi @mruberry,

I agree both of these can be refs only. I decided to make hypot as prim since it in C++ standard library, while heaviside, on the other side, does weird indexing like this:

def heaviside(a, b):
    ids = torch.eq(a, 0)
    ....
    a[ids] = b

is this a[ids] fully supported operation in refs?

I don't think we need these to be prims, do we, @Aidyn-A?

Hi @mruberry,

I agree both of these can be refs only. I decided to make hypot as prim since it in C++ standard library

Great point -- and it's a function in CUDA, too (although only the two element version, like PyTorch has). Following our previous reasoning, seems like hypot is correctly a prim, then! My mistake on that one.

while heaviside, on the other side, does weird indexing like this:

def heaviside(a, b):
    ids = torch.eq(a, 0)
    ....
    a[ids] = b

is this a[ids] fully supported operation in refs?

No we don't have reference implementations for indexing but I think we want to use torch.where, which is supported, instead?

No we don't have reference implementations for indexing but I think we want to use torch.where, which is supported, instead?

Right. We can use where. I will commit new changes. Thanks 👍

@mruberry, heaviside is now implemented as a reference, not a prim, and all tests are passing. Could you please take another look?

Hi @ngimel and @mruberry,

I fixed a tiny bug associated with where, now we can use it with two scalar arguments on the right hand side. Thanks to that I do not allocate tensors of zeros and ones in heaviside

Now it is ready for the review.
cc @ngimel @mruberry

@pytorchbot merge

@pytorchbot successfully started a merge job. Check the current status here

Hey @Aidyn-A.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.