Skip to content

Conversation

@wonjoo-wj
Copy link
Collaborator

@wonjoo-wj wonjoo-wj commented Jan 13, 2023

Attempt to fix #4414.


More details are in the issue above, but to quick a little more details on XLA's side -- the dynamo unit tests were failing after functionalization was enabled with error:

[2023-01-09 09:08:37,385] torch._dynamo.utils: [WARNING] Unsupported: meta converter nyi with fake tensor propagation.

Thanks to Ed's investigation, the root cause of this failure was narrowed down to torch._is_functional_tensor(t) resulting to True (at https://github.com/pytorch/pytorch/blob/master/torch/_subclasses/meta_utils.py#L473). Previous to enabling functionalization, this if statement results to False but now it equates to True since now XLA tensors are functional tensors. And because this equates to True, the code returns NotImplemented at https://github.com/pytorch/pytorch/blob/master/torch/_subclasses/meta_utils.py#L490. We don't want this, as we want this function to properly return a tensor by going into the code as lines https://github.com/pytorch/pytorch/blob/master/torch/_subclasses/meta_utils.py#L491-L508.

I've added a commit in upstream PR pytorch/pytorch#88787 (commit: pytorch/pytorch@c29cfe4) to exclude XLA tensors in the if statement to return NotImplemented.

After this fix, I can see that the dynamo tests passes locally. It might potentially break other tests, I'll wait for the CI to verify.

@wonjoo-wj
Copy link
Collaborator Author

Seems like it broke the test_narrow_copy_non_contiguous_xla test:

ERROR: test_narrow_copy_non_contiguous_xla (__main__.TestTorchDeviceTypeXLA)
RuntimeError: !schema.hasAnyAliasInfo() INTERNAL ASSERT FAILED at "/tmp/pytorch/aten/src/ATen/FunctionalizeFallbackKernel.cpp":32, please report a bug to PyTorch. mutating and aliasing ops should all have codegen'd kernels

Although this doesn't seem related at first. Looking into it.

@wonjoo-wj wonjoo-wj self-assigned this Jan 13, 2023
@wonjoo-wj wonjoo-wj force-pushed the functionalization-dynamo branch from 85bd2f5 to c57c53d Compare January 13, 2023 21:13
@wonjoo-wj
Copy link
Collaborator Author

Okay it seems like that test is a newly failing test with our functionalization branch, unrelated to this PR. Re-running the CI.

@wonjoo-wj wonjoo-wj force-pushed the functionalization-dynamo branch from c57c53d to f1f48e3 Compare January 13, 2023 21:21
@wonjoo-wj wonjoo-wj force-pushed the functionalization-dynamo branch from f1f48e3 to 7e366f7 Compare January 13, 2023 22:09
@alanwaketan alanwaketan force-pushed the functionalization branch 2 times, most recently from 9040f32 to 052ba74 Compare January 14, 2023 08:18
@alanwaketan
Copy link
Collaborator

@wonjoolee95 #4158 should be all green now. You can rebase and rerun the CI.

@wonjoo-wj wonjoo-wj force-pushed the functionalization-dynamo branch from e4ddb0d to 7d07913 Compare January 17, 2023 18:25
@wonjoo-wj
Copy link
Collaborator Author

Seems like the fix commit in the upstream PR (pytorch/pytorch@c29cfe4) is safe as it did not cause any new failures. This should be ready for a quick review, @alanwaketan. Thanks!

@wonjoo-wj wonjoo-wj marked this pull request as ready for review January 17, 2023 20:17
@wonjoo-wj wonjoo-wj requested a review from alanwaketan January 17, 2023 20:17
Copy link
Collaborator

@alanwaketan alanwaketan left a comment

Choose a reason for hiding this comment

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

LGTM. Please wait until the GPU tests finish.

Copy link
Collaborator

@JackCaoG JackCaoG left a comment

Choose a reason for hiding this comment

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

Thanks Wonjoo!

@wonjoo-wj
Copy link
Collaborator Author

Thanks! CI is green, merging this PR.

@wonjoo-wj wonjoo-wj merged commit 5cd6014 into functionalization Jan 17, 2023
alanwaketan pushed a commit that referenced this pull request Jan 25, 2023
alanwaketan pushed a commit that referenced this pull request Jan 26, 2023
wonjoo-wj added a commit that referenced this pull request Jan 26, 2023
wonjoo-wj added a commit that referenced this pull request Jan 31, 2023
yeounoh pushed a commit that referenced this pull request Feb 1, 2023
wonjoo-wj added a commit that referenced this pull request Feb 1, 2023
wonjoo-wj added a commit that referenced this pull request Feb 2, 2023
vanbasten23 pushed a commit that referenced this pull request Feb 2, 2023
vanbasten23 pushed a commit that referenced this pull request Feb 3, 2023
wonjoo-wj added a commit that referenced this pull request Feb 7, 2023
alanwaketan pushed a commit that referenced this pull request Feb 8, 2023
alanwaketan pushed a commit that referenced this pull request Feb 15, 2023
alanwaketan pushed a commit that referenced this pull request Feb 21, 2023
vanbasten23 pushed a commit that referenced this pull request Feb 23, 2023
alanwaketan pushed a commit that referenced this pull request Feb 24, 2023
vanbasten23 added a commit that referenced this pull request Feb 24, 2023
* Disable failing XLA tests

* Re-enable dynamo tests (#4454)

* Turn on keep going

* Re-enable more dynamic shape tests (#4558)

* fix size node ne op kind to be size_ne

* reenable test_nonzero_cast

* fix linter

* disable new failing test after rebase temporarily.

* fix linter

* [Functionalization] Fix ScatterReduce (#4576)

Summary:
ScatterReduce::reduce_ uses a unsafe type c10::string_view to store the
string. Replace it with std::string and re-enable all previous failing tests.

Test Plan:
TPU_LIBRARY_PATH=/home/ptxla/.local/lib/python3.8/site-packages/libtpu/libtpu.so PJRT_DEVICE=CPU test/cpp/build/test_ptxla --gtest_filter=AtenXlaTensorTest.TestScatterReduce*

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* code compiles

* wrote new test TestDynamicShapes.test_sizeMod and it succeeds.

* the first view_copy_symint test test_view_copy_symint_with_dyn_input_shape passed

* added two more tests

* run linter

* added a new failing test with negative shape

* fix all tests

* clean up

* clean up

* fix a small test failure.

* fix pr comments

* updated the tests for dynamic input and static input shape

* not support dynamic input and static input shape.

* fix linter

* fix pr comments

* fix build error

* resolve a rebase conflict

* run linter

* add one more test

* add one more test and it passes.

* fix linter

* remove experiemental test.

---------

Co-authored-by: Wonjoo Lee <wonjoo@google.com>
Co-authored-by: Jiewen Tan <jwtan@google.com>
vanbasten23 added a commit that referenced this pull request Feb 24, 2023
* Disable failing XLA tests

* Re-enable dynamo tests (#4454)

* Turn on keep going

* Re-enable more dynamic shape tests (#4558)

* fix size node ne op kind to be size_ne

* reenable test_nonzero_cast

* fix linter

* disable new failing test after rebase temporarily.

* fix linter

* [Functionalization] Fix ScatterReduce (#4576)

Summary:
ScatterReduce::reduce_ uses a unsafe type c10::string_view to store the
string. Replace it with std::string and re-enable all previous failing tests.

Test Plan:
TPU_LIBRARY_PATH=/home/ptxla/.local/lib/python3.8/site-packages/libtpu/libtpu.so PJRT_DEVICE=CPU test/cpp/build/test_ptxla --gtest_filter=AtenXlaTensorTest.TestScatterReduce*

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* code compiles

* wrote new test TestDynamicShapes.test_sizeMod and it succeeds.

* the first view_copy_symint test test_view_copy_symint_with_dyn_input_shape passed

* added two more tests

* run linter

* added a new failing test with negative shape

* fix all tests

* clean up

* clean up

* fix a small test failure.

* fix pr comments

* updated the tests for dynamic input and static input shape

* not support dynamic input and static input shape.

* fix linter

* fix pr comments

* fix build error

* resolve a rebase conflict

* run linter

* add one more test

* add one more test and it passes.

* fix linter

* remove experiemental test.

---------

Co-authored-by: Wonjoo Lee <wonjoo@google.com>
Co-authored-by: Jiewen Tan <jwtan@google.com>
alanwaketan pushed a commit that referenced this pull request Mar 1, 2023
yeounoh pushed a commit that referenced this pull request Mar 2, 2023
vanbasten23 added a commit that referenced this pull request Mar 7, 2023
Support dynamism on "transpose" operation. (#4606)

* added failing test_t_copy test

* made transpose work

* disable the backward pass.

* fix linter

Add dynamic shape test for unsqueeze_copy (#4608)

* added failing test

* test correctness.

Support dynamism on view_copy_symint (#4629)

* Disable failing XLA tests

* Re-enable dynamo tests (#4454)

* Turn on keep going

* Re-enable more dynamic shape tests (#4558)

* fix size node ne op kind to be size_ne

* reenable test_nonzero_cast

* fix linter

* disable new failing test after rebase temporarily.

* fix linter

* [Functionalization] Fix ScatterReduce (#4576)

Summary:
ScatterReduce::reduce_ uses a unsafe type c10::string_view to store the
string. Replace it with std::string and re-enable all previous failing tests.

Test Plan:
TPU_LIBRARY_PATH=/home/ptxla/.local/lib/python3.8/site-packages/libtpu/libtpu.so PJRT_DEVICE=CPU test/cpp/build/test_ptxla --gtest_filter=AtenXlaTensorTest.TestScatterReduce*

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* code compiles

* wrote new test TestDynamicShapes.test_sizeMod and it succeeds.

* the first view_copy_symint test test_view_copy_symint_with_dyn_input_shape passed

* added two more tests

* run linter

* added a new failing test with negative shape

* fix all tests

* clean up

* clean up

* fix a small test failure.

* fix pr comments

* updated the tests for dynamic input and static input shape

* not support dynamic input and static input shape.

* fix linter

* fix pr comments

* fix build error

* resolve a rebase conflict

* run linter

* add one more test

* add one more test and it passes.

* fix linter

* remove experiemental test.

---------

Co-authored-by: Wonjoo Lee <wonjoo@google.com>
Co-authored-by: Jiewen Tan <jwtan@google.com>

fix a merge error

fix other merge error during rebase.
vanbasten23 added a commit that referenced this pull request Mar 8, 2023
Support dynamism on "transpose" operation. (#4606)

* added failing test_t_copy test

* made transpose work

* disable the backward pass.

* fix linter

Add dynamic shape test for unsqueeze_copy (#4608)

* added failing test

* test correctness.

Support dynamism on view_copy_symint (#4629)

* Disable failing XLA tests

* Re-enable dynamo tests (#4454)

* Turn on keep going

* Re-enable more dynamic shape tests (#4558)

* fix size node ne op kind to be size_ne

* reenable test_nonzero_cast

* fix linter

* disable new failing test after rebase temporarily.

* fix linter

* [Functionalization] Fix ScatterReduce (#4576)

Summary:
ScatterReduce::reduce_ uses a unsafe type c10::string_view to store the
string. Replace it with std::string and re-enable all previous failing tests.

Test Plan:
TPU_LIBRARY_PATH=/home/ptxla/.local/lib/python3.8/site-packages/libtpu/libtpu.so PJRT_DEVICE=CPU test/cpp/build/test_ptxla --gtest_filter=AtenXlaTensorTest.TestScatterReduce*

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* code compiles

* wrote new test TestDynamicShapes.test_sizeMod and it succeeds.

* the first view_copy_symint test test_view_copy_symint_with_dyn_input_shape passed

* added two more tests

* run linter

* added a new failing test with negative shape

* fix all tests

* clean up

* clean up

* fix a small test failure.

* fix pr comments

* updated the tests for dynamic input and static input shape

* not support dynamic input and static input shape.

* fix linter

* fix pr comments

* fix build error

* resolve a rebase conflict

* run linter

* add one more test

* add one more test and it passes.

* fix linter

* remove experiemental test.

---------

Co-authored-by: Wonjoo Lee <wonjoo@google.com>
Co-authored-by: Jiewen Tan <jwtan@google.com>

fix a merge error

fix other merge error during rebase.
vanbasten23 added a commit that referenced this pull request Mar 9, 2023
* Support dynamism on "transpose", unsqueeze_copy, and view_copy_symint.

Support dynamism on "transpose" operation. (#4606)

* added failing test_t_copy test

* made transpose work

* disable the backward pass.

* fix linter

Add dynamic shape test for unsqueeze_copy (#4608)

* added failing test

* test correctness.

Support dynamism on view_copy_symint (#4629)

* Disable failing XLA tests

* Re-enable dynamo tests (#4454)

* Turn on keep going

* Re-enable more dynamic shape tests (#4558)

* fix size node ne op kind to be size_ne

* reenable test_nonzero_cast

* fix linter

* disable new failing test after rebase temporarily.

* fix linter

* [Functionalization] Fix ScatterReduce (#4576)

Summary:
ScatterReduce::reduce_ uses a unsafe type c10::string_view to store the
string. Replace it with std::string and re-enable all previous failing tests.

Test Plan:
TPU_LIBRARY_PATH=/home/ptxla/.local/lib/python3.8/site-packages/libtpu/libtpu.so PJRT_DEVICE=CPU test/cpp/build/test_ptxla --gtest_filter=AtenXlaTensorTest.TestScatterReduce*

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* code compiles

* wrote new test TestDynamicShapes.test_sizeMod and it succeeds.

* the first view_copy_symint test test_view_copy_symint_with_dyn_input_shape passed

* added two more tests

* run linter

* added a new failing test with negative shape

* fix all tests

* clean up

* clean up

* fix a small test failure.

* fix pr comments

* updated the tests for dynamic input and static input shape

* not support dynamic input and static input shape.

* fix linter

* fix pr comments

* fix build error

* resolve a rebase conflict

* run linter

* add one more test

* add one more test and it passes.

* fix linter

* remove experiemental test.

---------

Co-authored-by: Wonjoo Lee <wonjoo@google.com>
Co-authored-by: Jiewen Tan <jwtan@google.com>

fix a merge error

fix other merge error during rebase.

* Track type of SymNode for XLASymNodeImpl::mod (#4732)
mateuszlewko pushed a commit that referenced this pull request Mar 15, 2023
* Support dynamism on "transpose", unsqueeze_copy, and view_copy_symint.

Support dynamism on "transpose" operation. (#4606)

* added failing test_t_copy test

* made transpose work

* disable the backward pass.

* fix linter

Add dynamic shape test for unsqueeze_copy (#4608)

* added failing test

* test correctness.

Support dynamism on view_copy_symint (#4629)

* Disable failing XLA tests

* Re-enable dynamo tests (#4454)

* Turn on keep going

* Re-enable more dynamic shape tests (#4558)

* fix size node ne op kind to be size_ne

* reenable test_nonzero_cast

* fix linter

* disable new failing test after rebase temporarily.

* fix linter

* [Functionalization] Fix ScatterReduce (#4576)

Summary:
ScatterReduce::reduce_ uses a unsafe type c10::string_view to store the
string. Replace it with std::string and re-enable all previous failing tests.

Test Plan:
TPU_LIBRARY_PATH=/home/ptxla/.local/lib/python3.8/site-packages/libtpu/libtpu.so PJRT_DEVICE=CPU test/cpp/build/test_ptxla --gtest_filter=AtenXlaTensorTest.TestScatterReduce*

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* code compiles

* wrote new test TestDynamicShapes.test_sizeMod and it succeeds.

* the first view_copy_symint test test_view_copy_symint_with_dyn_input_shape passed

* added two more tests

* run linter

* added a new failing test with negative shape

* fix all tests

* clean up

* clean up

* fix a small test failure.

* fix pr comments

* updated the tests for dynamic input and static input shape

* not support dynamic input and static input shape.

* fix linter

* fix pr comments

* fix build error

* resolve a rebase conflict

* run linter

* add one more test

* add one more test and it passes.

* fix linter

* remove experiemental test.

---------

Co-authored-by: Wonjoo Lee <wonjoo@google.com>
Co-authored-by: Jiewen Tan <jwtan@google.com>

fix a merge error

fix other merge error during rebase.

* Track type of SymNode for XLASymNodeImpl::mod (#4732)
ManfeiBai pushed a commit to ManfeiBai/PyTorchXLA that referenced this pull request Mar 29, 2023
* Support dynamism on "transpose", unsqueeze_copy, and view_copy_symint.

Support dynamism on "transpose" operation. (pytorch#4606)

* added failing test_t_copy test

* made transpose work

* disable the backward pass.

* fix linter

Add dynamic shape test for unsqueeze_copy (pytorch#4608)

* added failing test

* test correctness.

Support dynamism on view_copy_symint (pytorch#4629)

* Disable failing XLA tests

* Re-enable dynamo tests (pytorch#4454)

* Turn on keep going

* Re-enable more dynamic shape tests (pytorch#4558)

* fix size node ne op kind to be size_ne

* reenable test_nonzero_cast

* fix linter

* disable new failing test after rebase temporarily.

* fix linter

* [Functionalization] Fix ScatterReduce (pytorch#4576)

Summary:
ScatterReduce::reduce_ uses a unsafe type c10::string_view to store the
string. Replace it with std::string and re-enable all previous failing tests.

Test Plan:
TPU_LIBRARY_PATH=/home/ptxla/.local/lib/python3.8/site-packages/libtpu/libtpu.so PJRT_DEVICE=CPU test/cpp/build/test_ptxla --gtest_filter=AtenXlaTensorTest.TestScatterReduce*

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* code compiles

* wrote new test TestDynamicShapes.test_sizeMod and it succeeds.

* the first view_copy_symint test test_view_copy_symint_with_dyn_input_shape passed

* added two more tests

* run linter

* added a new failing test with negative shape

* fix all tests

* clean up

* clean up

* fix a small test failure.

* fix pr comments

* updated the tests for dynamic input and static input shape

* not support dynamic input and static input shape.

* fix linter

* fix pr comments

* fix build error

* resolve a rebase conflict

* run linter

* add one more test

* add one more test and it passes.

* fix linter

* remove experiemental test.

---------

Co-authored-by: Wonjoo Lee <wonjoo@google.com>
Co-authored-by: Jiewen Tan <jwtan@google.com>

fix a merge error

fix other merge error during rebase.

* Track type of SymNode for XLASymNodeImpl::mod (pytorch#4732)
ManfeiBai pushed a commit to ManfeiBai/PyTorchXLA that referenced this pull request Mar 29, 2023
* Support dynamism on "transpose", unsqueeze_copy, and view_copy_symint.

Support dynamism on "transpose" operation. (pytorch#4606)

* added failing test_t_copy test

* made transpose work

* disable the backward pass.

* fix linter

Add dynamic shape test for unsqueeze_copy (pytorch#4608)

* added failing test

* test correctness.

Support dynamism on view_copy_symint (pytorch#4629)

* Disable failing XLA tests

* Re-enable dynamo tests (pytorch#4454)

* Turn on keep going

* Re-enable more dynamic shape tests (pytorch#4558)

* fix size node ne op kind to be size_ne

* reenable test_nonzero_cast

* fix linter

* disable new failing test after rebase temporarily.

* fix linter

* [Functionalization] Fix ScatterReduce (pytorch#4576)

Summary:
ScatterReduce::reduce_ uses a unsafe type c10::string_view to store the
string. Replace it with std::string and re-enable all previous failing tests.

Test Plan:
TPU_LIBRARY_PATH=/home/ptxla/.local/lib/python3.8/site-packages/libtpu/libtpu.so PJRT_DEVICE=CPU test/cpp/build/test_ptxla --gtest_filter=AtenXlaTensorTest.TestScatterReduce*

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* code compiles

* wrote new test TestDynamicShapes.test_sizeMod and it succeeds.

* the first view_copy_symint test test_view_copy_symint_with_dyn_input_shape passed

* added two more tests

* run linter

* added a new failing test with negative shape

* fix all tests

* clean up

* clean up

* fix a small test failure.

* fix pr comments

* updated the tests for dynamic input and static input shape

* not support dynamic input and static input shape.

* fix linter

* fix pr comments

* fix build error

* resolve a rebase conflict

* run linter

* add one more test

* add one more test and it passes.

* fix linter

* remove experiemental test.

---------

Co-authored-by: Wonjoo Lee <wonjoo@google.com>
Co-authored-by: Jiewen Tan <jwtan@google.com>

fix a merge error

fix other merge error during rebase.

* Track type of SymNode for XLASymNodeImpl::mod (pytorch#4732)
vanbasten23 pushed a commit that referenced this pull request Apr 29, 2023
vanbasten23 added a commit that referenced this pull request Apr 29, 2023
* Disable failing XLA tests

* Re-enable dynamo tests (#4454)

* Turn on keep going

* Re-enable more dynamic shape tests (#4558)

* fix size node ne op kind to be size_ne

* reenable test_nonzero_cast

* fix linter

* disable new failing test after rebase temporarily.

* fix linter

* [Functionalization] Fix ScatterReduce (#4576)

Summary:
ScatterReduce::reduce_ uses a unsafe type c10::string_view to store the
string. Replace it with std::string and re-enable all previous failing tests.

Test Plan:
TPU_LIBRARY_PATH=/home/ptxla/.local/lib/python3.8/site-packages/libtpu/libtpu.so PJRT_DEVICE=CPU test/cpp/build/test_ptxla --gtest_filter=AtenXlaTensorTest.TestScatterReduce*

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* code compiles

* wrote new test TestDynamicShapes.test_sizeMod and it succeeds.

* the first view_copy_symint test test_view_copy_symint_with_dyn_input_shape passed

* added two more tests

* run linter

* added a new failing test with negative shape

* fix all tests

* clean up

* clean up

* fix a small test failure.

* fix pr comments

* updated the tests for dynamic input and static input shape

* not support dynamic input and static input shape.

* fix linter

* fix pr comments

* fix build error

* resolve a rebase conflict

* run linter

* add one more test

* add one more test and it passes.

* fix linter

* remove experiemental test.

---------

Co-authored-by: Wonjoo Lee <wonjoo@google.com>
Co-authored-by: Jiewen Tan <jwtan@google.com>
vanbasten23 pushed a commit that referenced this pull request May 12, 2023
vanbasten23 added a commit that referenced this pull request May 12, 2023
* Disable failing XLA tests

* Re-enable dynamo tests (#4454)

* Turn on keep going

* Re-enable more dynamic shape tests (#4558)

* fix size node ne op kind to be size_ne

* reenable test_nonzero_cast

* fix linter

* disable new failing test after rebase temporarily.

* fix linter

* [Functionalization] Fix ScatterReduce (#4576)

Summary:
ScatterReduce::reduce_ uses a unsafe type c10::string_view to store the
string. Replace it with std::string and re-enable all previous failing tests.

Test Plan:
TPU_LIBRARY_PATH=/home/ptxla/.local/lib/python3.8/site-packages/libtpu/libtpu.so PJRT_DEVICE=CPU test/cpp/build/test_ptxla --gtest_filter=AtenXlaTensorTest.TestScatterReduce*

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* code compiles

* wrote new test TestDynamicShapes.test_sizeMod and it succeeds.

* the first view_copy_symint test test_view_copy_symint_with_dyn_input_shape passed

* added two more tests

* run linter

* added a new failing test with negative shape

* fix all tests

* clean up

* clean up

* fix a small test failure.

* fix pr comments

* updated the tests for dynamic input and static input shape

* not support dynamic input and static input shape.

* fix linter

* fix pr comments

* fix build error

* resolve a rebase conflict

* run linter

* add one more test

* add one more test and it passes.

* fix linter

* remove experiemental test.

---------

Co-authored-by: Wonjoo Lee <wonjoo@google.com>
Co-authored-by: Jiewen Tan <jwtan@google.com>
vanbasten23 pushed a commit that referenced this pull request Jun 21, 2023
vanbasten23 added a commit that referenced this pull request Jun 21, 2023
* Disable failing XLA tests

* Re-enable dynamo tests (#4454)

* Turn on keep going

* Re-enable more dynamic shape tests (#4558)

* fix size node ne op kind to be size_ne

* reenable test_nonzero_cast

* fix linter

* disable new failing test after rebase temporarily.

* fix linter

* [Functionalization] Fix ScatterReduce (#4576)

Summary:
ScatterReduce::reduce_ uses a unsafe type c10::string_view to store the
string. Replace it with std::string and re-enable all previous failing tests.

Test Plan:
TPU_LIBRARY_PATH=/home/ptxla/.local/lib/python3.8/site-packages/libtpu/libtpu.so PJRT_DEVICE=CPU test/cpp/build/test_ptxla --gtest_filter=AtenXlaTensorTest.TestScatterReduce*

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* code compiles

* wrote new test TestDynamicShapes.test_sizeMod and it succeeds.

* the first view_copy_symint test test_view_copy_symint_with_dyn_input_shape passed

* added two more tests

* run linter

* added a new failing test with negative shape

* fix all tests

* clean up

* clean up

* fix a small test failure.

* fix pr comments

* updated the tests for dynamic input and static input shape

* not support dynamic input and static input shape.

* fix linter

* fix pr comments

* fix build error

* resolve a rebase conflict

* run linter

* add one more test

* add one more test and it passes.

* fix linter

* remove experiemental test.

---------

Co-authored-by: Wonjoo Lee <wonjoo@google.com>
Co-authored-by: Jiewen Tan <jwtan@google.com>
vanbasten23 pushed a commit that referenced this pull request Jun 24, 2023
vanbasten23 added a commit that referenced this pull request Jun 24, 2023
* Disable failing XLA tests

* Re-enable dynamo tests (#4454)

* Turn on keep going

* Re-enable more dynamic shape tests (#4558)

* fix size node ne op kind to be size_ne

* reenable test_nonzero_cast

* fix linter

* disable new failing test after rebase temporarily.

* fix linter

* [Functionalization] Fix ScatterReduce (#4576)

Summary:
ScatterReduce::reduce_ uses a unsafe type c10::string_view to store the
string. Replace it with std::string and re-enable all previous failing tests.

Test Plan:
TPU_LIBRARY_PATH=/home/ptxla/.local/lib/python3.8/site-packages/libtpu/libtpu.so PJRT_DEVICE=CPU test/cpp/build/test_ptxla --gtest_filter=AtenXlaTensorTest.TestScatterReduce*

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* code compiles

* wrote new test TestDynamicShapes.test_sizeMod and it succeeds.

* the first view_copy_symint test test_view_copy_symint_with_dyn_input_shape passed

* added two more tests

* run linter

* added a new failing test with negative shape

* fix all tests

* clean up

* clean up

* fix a small test failure.

* fix pr comments

* updated the tests for dynamic input and static input shape

* not support dynamic input and static input shape.

* fix linter

* fix pr comments

* fix build error

* resolve a rebase conflict

* run linter

* add one more test

* add one more test and it passes.

* fix linter

* remove experiemental test.

---------

Co-authored-by: Wonjoo Lee <wonjoo@google.com>
Co-authored-by: Jiewen Tan <jwtan@google.com>
vanbasten23 pushed a commit that referenced this pull request Jun 27, 2023
vanbasten23 added a commit that referenced this pull request Jun 27, 2023
* Disable failing XLA tests

* Re-enable dynamo tests (#4454)

* Turn on keep going

* Re-enable more dynamic shape tests (#4558)

* fix size node ne op kind to be size_ne

* reenable test_nonzero_cast

* fix linter

* disable new failing test after rebase temporarily.

* fix linter

* [Functionalization] Fix ScatterReduce (#4576)

Summary:
ScatterReduce::reduce_ uses a unsafe type c10::string_view to store the
string. Replace it with std::string and re-enable all previous failing tests.

Test Plan:
TPU_LIBRARY_PATH=/home/ptxla/.local/lib/python3.8/site-packages/libtpu/libtpu.so PJRT_DEVICE=CPU test/cpp/build/test_ptxla --gtest_filter=AtenXlaTensorTest.TestScatterReduce*

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* Turn on keep going

* Revert "Turn on keep going"

This reverts commit 224bdf3.

* code compiles

* wrote new test TestDynamicShapes.test_sizeMod and it succeeds.

* the first view_copy_symint test test_view_copy_symint_with_dyn_input_shape passed

* added two more tests

* run linter

* added a new failing test with negative shape

* fix all tests

* clean up

* clean up

* fix a small test failure.

* fix pr comments

* updated the tests for dynamic input and static input shape

* not support dynamic input and static input shape.

* fix linter

* fix pr comments

* fix build error

* resolve a rebase conflict

* run linter

* add one more test

* add one more test and it passes.

* fix linter

* remove experiemental test.

---------

Co-authored-by: Wonjoo Lee <wonjoo@google.com>
Co-authored-by: Jiewen Tan <jwtan@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants