Skip to content

Conversation

apbose
Copy link
Collaborator

@apbose apbose commented Jul 17, 2025

This PR-

  1. Addresses 🐛 [Bug] index and index_put don't support dynamic shapes #3647 for index
  2. Adds a test case for dynamic index

@meta-cla meta-cla bot added the cla signed label Jul 17, 2025
@github-actions github-actions bot added component: tests Issues re: Tests component: conversion Issues re: Conversion stage component: api [Python] Issues re: Python API component: dynamo Issues relating to the `torch.compile` or `torch._dynamo.export` paths labels Jul 17, 2025
@github-actions github-actions bot requested a review from peri044 July 17, 2025 18:15
@peri044
Copy link
Collaborator

peri044 commented Jul 24, 2025

The index dtype validator doesn't support bool type

(Pdb) node.args
(_reshape_copy_597, [eq_1427])
(Pdb) node.args
(_reshape_copy_597, [eq_1427])
(Pdb) node.args[0].dtype
*** AttributeError: 'Node' object has no attribute 'dtype'
(Pdb) node.args[0].meta['val']
FakeTensor(..., device='cuda:0', size=(s43*s53, 2048), dtype=torch.float16)
(Pdb) node.args[0].meta['val'].dtype
torch.float16
(Pdb) node.args[1][0].meta['val'].dtype
torch.bool

The inputs to index are FP16 and bool. In this case, converter is not being invoked and falls back to PyTorch

@apbose
Copy link
Collaborator Author

apbose commented Jul 24, 2025

oh ok, looks like in https://github.com/zdevito/ATen/blob/master/aten/src/ATen/native/native_functions.yaml#L1324 tensor indices as bool should denote mask then. I think a non zero layer to get the corresponding indices should work. I will push in the change

@apbose apbose force-pushed the abose/index_dynamic_fix branch from e5aae73 to a14fc4c Compare July 31, 2025 20:49
@github-actions github-actions bot added the component: converters Issues re: Specific op converters label Jul 31, 2025
@apbose apbose force-pushed the abose/index_dynamic_fix branch from 1834500 to 663fbe3 Compare July 31, 2025 23:02
Copy link
Collaborator

@lanluo-nvidia lanluo-nvidia left a comment

Choose a reason for hiding this comment

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

One comment, please take a look, others LGTM

@apbose apbose force-pushed the abose/index_dynamic_fix branch 3 times, most recently from 19c0703 to a43302c Compare August 26, 2025 05:59
@apbose apbose force-pushed the abose/index_dynamic_fix branch from a43302c to 3a2969c Compare August 26, 2025 17:32
@apbose
Copy link
Collaborator Author

apbose commented Aug 28, 2025

I think the tests are passing related to this test. @peri044 could you please review?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed component: api [Python] Issues re: Python API component: conversion Issues re: Conversion stage component: converters Issues re: Specific op converters component: dynamo Issues relating to the `torch.compile` or `torch._dynamo.export` paths component: tests Issues re: Tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants