Skip to content

[Functionalization] Remove CreateAsStridedViewInfo#5084

Merged
alanwaketan merged 4 commits intomasterfrom
alanwaketan/as_strided_2
Jun 14, 2023
Merged

[Functionalization] Remove CreateAsStridedViewInfo#5084
alanwaketan merged 4 commits intomasterfrom
alanwaketan/as_strided_2

Conversation

@alanwaketan
Copy link
Copy Markdown
Collaborator

@alanwaketan alanwaketan commented May 25, 2023

Summary:
This PR removes the usage of CreateAsStridedViewInfo in tensor_methods::as_strided.

Test Plan:
PJRT_DEVICE=CPU python ../test/test_view_ops.py -v -k TestViewOpsXLA.test_as_strided

@vanbasten23
Copy link
Copy Markdown
Collaborator

vanbasten23 commented May 25, 2023

Thanks a lot. Since TORCH.AS_STRIDED "Create a view of an existing torch.Tensor" and we have already enabled functionalization, I wonder why is this function still called.

@alanwaketan
Copy link
Copy Markdown
Collaborator Author

alanwaketan commented May 26, 2023

Thanks a lot. Since TORCH.AS_STRIDED "Create a view of an existing torch.Tensor" and we have already enabled functionalization, I wonder why is this function still called.

Well, we can rename it to as_strided_copy but I guess it doesn't matter for now... Will resolve the CI issue later.

@vanbasten23 vanbasten23 mentioned this pull request Jun 9, 2023
Merged
@alanwaketan alanwaketan force-pushed the alanwaketan/as_strided_2 branch from 36bdb18 to 7f980ad Compare June 14, 2023 18:31
@alanwaketan
[Functionalization] Remove CreateAsStridedViewInfo
48641f7 
Summary:
This PR removes the usage of CreateAsStridedViewInfo in
tensor_methods::as_strided.

Test Plan:
PJRT_DEVICE=CPU python ../test/test_view_ops.py -v -k TestViewOpsXLA.test_as_strided
@alanwaketan
linters
87d3159
@alanwaketan
Restore the previous behavior for XLA_DISABLE_FUNCTIONALIZATION
57a825e
@alanwaketan
Fix the build
f948036
@alanwaketan alanwaketan force-pushed the alanwaketan/as_strided_2 branch from 42aeb4b to f948036 Compare June 14, 2023 19:59
vanbasten23
vanbasten23 reviewed Jun 14, 2023
View reviewed changes
Comment thread torch_xla/csrc/tensor_methods.cpp Outdated
std::vector<int64_t> stride,
c10::optional<int64_t> storage_offset) {
// See Note: [Disabling functionalization]
if (xla::sys_util::GetEnvBool("XLA_DISABLE_FUNCTIONALIZATION", false)) {
Copy link
Copy Markdown
Collaborator

@vanbasten23 vanbasten23 Jun 14, 2023

Choose a reason for hiding this comment

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

I'm trying to figure out how you fix the test. Right now, which branch is being run, the if block or the else block?

Copy link
Copy Markdown
Collaborator Author

@alanwaketan alanwaketan Jun 14, 2023

Choose a reason for hiding this comment

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

By default the flag is off, we have a dedicated run of test_operations.py with the flag on.

@alanwaketan alanwaketan merged commit d438315 into master Jun 14, 2023
@alanwaketan
Copy link
Copy Markdown
Collaborator Author

alanwaketan commented Jun 14, 2023

Thanks everyone for approving the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vanbasten23 vanbasten23 vanbasten23 approved these changes

@wonjoo-wj wonjoo-wj wonjoo-wj approved these changes

@JackCaoG JackCaoG JackCaoG approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@alanwaketan @vanbasten23 @wonjoo-wj @JackCaoG