Skip to content

Conversation

@vishwakftw
Copy link
Contributor

I used

sed -i 's/at::rand(\({.*}\), at::TensorOptions(at::kFloat))/GetTestTensor(\1)/g' test_aten_xla_tensor.cpp

@vishwakftw
Copy link
Contributor Author

For formatting I used

clang-format -style=Google test_aten_xla_tensor.cpp | diff test_aten_xla_tensor.cpp -

to resolve formatting issues.

Copy link
Collaborator

@dlibenzi dlibenzi left a comment

Choose a reason for hiding this comment

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

I noticed the inconsistency yesterday, but honestly, GetTestTensor() really does nothing special (not even setting the XLA device).
It first born with the idea of setting the XLA device, but the new design has ForEachDevice(), so that's just an empty shell.
I'd eventually go the other way around, as here we are already doing a bunch of at:: API calls, and we can be explicit about at::rand() as well.

@vishwakftw
Copy link
Contributor Author

That sounds reasonable, I'll revert the changes and remove GetTestTensor.

@vishwakftw vishwakftw changed the title Use GetTestTensor for at::rand Use at::rand for GetTestTensor, remove GetTestTensor Mar 3, 2019
@vishwakftw
Copy link
Contributor Author

The below command returns nothing to stdout:

clang-format -style=Google test_aten_xla_tensor.cpp | diff test_aten_xla_tensor.cpp -

I believe the file is formatted as required.

@dlibenzi dlibenzi merged commit 5bf2fe7 into pytorch:master Mar 3, 2019
@vishwakftw vishwakftw deleted the replace-rand-GetTestTensor branch March 3, 2019 16:55
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.

2 participants