OpInfo: split, split_with_sizes #58184
Conversation
💊 CI failures summary and remediationsAs of commit 0554930 (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions to the (internal) Dr. CI Users group. |
| (True, 'aten::split_with_sizes'), [1]), | ||
| ('split_with_sizes', (S, S, S), ([int(S / 3), S - int(S / 3) * 2, int(S / 3)],), '', (True,)), | ||
| ('split_with_sizes', (S, S, S), ([int(S / 3), S - int(S / 3), 0],), 'size_0', (True, )), | ||
| ('split_with_sizes', (S, S, S), ([int(S / 3), S - int(S / 3) * 2, int(S / 3)],), 'dim', (True, ), [1]), |
There was a problem hiding this comment.
I assume that this cases was supposed to add a dim as well to the argument (otherwise it is the same as first). Have added one in sample_inputs_split_with_sizes.
There was a problem hiding this comment.
I think the test generator from method_tests understood the dim argument and would programmatically generate some options for it
There was a problem hiding this comment.
I don't think that is the case.
For checking, I had added the following snippet after the linked line.
if 'split_with' in test_name:
print(test_name, new_args)Line 5390 in 82d7149
Output for the given sample was (not all args are same),
test_split_with_sizes_dim_neg0 ([1, 3, 1],)
test_split_with_sizes_dim ([1, 3, 1],)
test_split_with_sizes_dim_neg0_complex ([1, 3, 1],)
test_split_with_sizes_dim_complex ([1, 3, 1],)
There was a problem hiding this comment.
Interesting; thanks for investigating. Good thing soon method_tests() will be gone and we won't have to wonder at its mysteries anymore
kshitij12345
changed the title
| variant_test_name='list_args', | ||
| dtypes=all_types_and_complex_and(torch.bfloat16, torch.half, torch.bool), | ||
| sample_inputs_func=partial(sample_inputs_split, list_args=True), | ||
| supports_out=False), |
There was a problem hiding this comment.
Looking at these entries,
('split', (S, S, S), ([int(S / 3), S - int(S / 3) * 2, int(S / 3)],), 'size_list',
(True, 'aten::split_with_sizes')),
('split', (S, S, S), ([int(S / 2), S - int(S / 2) * 2, int(S / 2)], 2), 'size_list_dim',
(True, 'aten::split_with_sizes'), [1]),
and
tried adding to the entry with list_args
assert_autodiffed=True,
autodiff_nonfusible_nodes=['aten::split_with_sizes']but jit consistency test failed with,
Failure in testing nodes' autodifferentiation. One or more nodes were expected to be autodiffed, but were not found in specified fusible/nonfusible DifferentiableGraph groups.
Specifically:
['aten::split_with_sizes'] were not in one of the DifferentiableGraphs when they were expected to be. Did you intend for these nodes to be autodiffed? If not, remove them from the list of nonfusible nodes.
There was a problem hiding this comment.
Great attention to detail as always, @kshitij12345
I'm not sure what's going on here. I made a sample program:
import torch
S = 5
t = torch.randn((S, S, S), requires_grad=True)
def foo(t):
S = 5
return t.split([int(S / 2), S - int(S / 2) * 2, int(S / 2)])
scripted_foo = torch.jit.script(foo)
scripted_foo.graph_for(t)
traced_foo = torch.jit.trace(foo, (t,))
traced_foo.graph
And I can see "split" becomes "split_with_sizes" in the traced graph, but not the scripted graph, and it doesn't look like it's part of a differentiable subgraph in either case.
Looking at the code it does seem like there's some special analysis that might convert split to split_with_sizes, but I can't see it being triggered, either:
I wonder if the autodifferentiation part of method_tests() is even running? Would you submit a PR that just deletes the (True, 'aten::split_with_sizes') from split and see if anything fails? I tried to find the tests for it in the code and couldn't spot them.
There was a problem hiding this comment.
Locally verified:
With the following diff
+ ('split', (S, S, S), ([int(S / 3), S - int(S / 3) * 2, int(S / 3)],), 'size_list',),
+ ('split', (S, S, S), ([int(S / 2), S - int(S / 2) * 2, int(S / 2)], 2), 'size_list_dim', (), [1]),The tests pass locally.
python test/test_autograd.py -k _split
/home/kshiteej/.conda/envs/pytorch-cuda-dev/lib/python3.8/site-packages/torch/backends/cudnn/__init__.py:73: UserWarning: PyTorch was compiled without cuDNN/MIOpen support. To use cuDNN/MIOpen, rebuild PyTorch making sure the library is visible to the build system.
warnings.warn(
................../home/kshiteej/.conda/envs/pytorch-cuda-dev/lib/python3.8/site-packages/torch/backends/cudnn/__init__.py:73: UserWarning: PyTorch was compiled without cuDNN/MIOpen support. To use cuDNN/MIOpen, rebuild PyTorch making sure the library is visible to the build system.
warnings.warn(
..................ssssssssssssssssss
----------------------------------------------------------------------
Ran 54 tests in 2.998s
OK (skipped=18)
Will submit a PR to verify the same on CI.
Thanks for digging into it and the suggestion :)
kshitij12345
changed the title
agolynski
added
the
triaged
Codecov Report
@@ Coverage Diff @@
## master #58184 +/- ##
==========================================
- Coverage 76.81% 76.79% -0.02%
==========================================
Files 1986 1986
Lines 198192 198208 +16
==========================================
- Hits 152232 152212 -20
- Misses 45960 45996 +36 |
|
@mruberry gentle ping :) |
|
@mruberry has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
5 participants
Reference: #54261