Skip to content

[LTC] Replace XLA_VALUE_METRIC with TORCH_LAZY_VALUE_METRIC - #4245

Merged
alanwaketan merged 5 commits into
masterfrom
alanwaketan/metrics
Nov 28, 2022
Merged

alanwaketan merged 5 commits into
masterfrom
alanwaketan/metrics

Conversation

@alanwaketan

@alanwaketan alanwaketan commented Nov 24, 2022 •

Copy link
Copy Markdown
Collaborator

Summary:
This pull request replaces all XLA_VALUE_METRIC in pytorch/xla with upstream
TORCH_LAZY_VALUE_METRIC.

This is paired with pytorch/pytorch#89606.

Test Plan:
PJRT_DEVICE=CPU python test/test_metrics.py

@alanwaketan alanwaketan added the tracing Lazy Tensor tracing label Nov 24, 2022
@alanwaketan alanwaketan self-assigned this Nov 24, 2022
@alanwaketan
alanwaketan marked this pull request as draft November 24, 2022 01:15
@alanwaketan alanwaketan changed the title Alanwaketan/metrics Nov 24, 2022
@alanwaketan
alanwaketan marked this pull request as ready for review November 24, 2022 01:22
@alanwaketan
alanwaketan requested a review from JackCaoG November 24, 2022 01:22
Summary:
This pull request replaces all XLA_VALUE_METRIC in pytorch/xla with upstream
TORCH_LAZY_VALUE_METRIC.

Test Plan:
PJRT_DEVICE=CPU python test/test_metrics.py
Comment thread test/test_metrics.py
# TODO(jwtan): Add test to cover TrimIrGraph, SyncTensorsToData, TransferToServerAsync, IrValueTensorToXlaData
xla_device = xm.xla_device()
t1 = torch.tensor(1456, device=xla_device)
t1 = torch.tensor(2077, device=xla_device)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why this change?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It looks like somehow we cache the device data in the server? If we reuse the same value for the scalar tensor, TransferToServerTime will be omitted in some of the tests.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

oh right.

@JackCaoG JackCaoG left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

pytorchmergebot pushed a commit to pytorch/pytorch that referenced this pull request Nov 28, 2022
Summary:
This change allow MetricsArena to ResetMetrics too. And then rename Reset to ResetCounters given that's what it does for real.

This matches pytorch/xla#4109, and is paired with pytorch/xla#4245.

Test Plan:
CI.

Pull Request resolved: #89606
Approved by: https://github.com/JackCaoG
@alanwaketan
alanwaketan merged commit 09498a8 into master Nov 28, 2022
alanwaketan added a commit that referenced this pull request Nov 28, 2022
Summary:
This patch just simply replaces XLA_TIMED with TORCH_LAZY_TIMED.
TORCH_LAZY_TIMED means timed metrics. Most of the work has been done
by #4245.

Test Plan:
PJRT_DEVICE=CPU python test/test_metrics.py
alanwaketan added a commit that referenced this pull request Nov 29, 2022
Summary:
This patch just simply replaces XLA_TIMED with TORCH_LAZY_TIMED.
TORCH_LAZY_TIMED means timed metrics. Most of the work has been done
by #4245.

Test Plan:
PJRT_DEVICE=CPU python test/test_metrics.py
kulinseth pushed a commit to kulinseth/pytorch that referenced this pull request Dec 10, 2022
Summary:
This change allow MetricsArena to ResetMetrics too. And then rename Reset to ResetCounters given that's what it does for real.

This matches pytorch/xla#4109, and is paired with pytorch/xla#4245.

Test Plan:
CI.

Pull Request resolved: pytorch#89606
Approved by: https://github.com/JackCaoG
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

REMOVE_TORCH_PIN tracing Lazy Tensor tracing

2 participants