Skip to content

Incorporate compilation environment in hash#5803

Merged
jonb377 merged 4 commits intomasterfrom
jonbolin/comp-env-hash
Dec 6, 2023
Merged

Incorporate compilation environment in hash#5803
jonb377 merged 4 commits intomasterfrom
jonbolin/comp-env-hash

Conversation

@jonb377
Copy link
Copy Markdown
Collaborator

@jonb377 jonb377 commented Nov 15, 2023

Include the compilation environment in the graph hash. This will ensure that options which impact the compilation result will change the hash, so the same traced IR graph will result in a different hash when the compilation environment is different.

Currently, the following env vars must be hashed: XLA_FLAGS, LIBTPU_INIT_ARGS, and TPU_MEGACORE. All env vars which impact the hash must be included in env_hash.cc, otherwise there can be spurious cache hits!

This PR builds on #5800, but it doesn't depend on it and can be landed separately.

Comment thread torch_xla/csrc/xla_graph_executor.cpp
@jonb377 jonb377 mentioned this pull request Nov 15, 2023
Comment thread torch_xla/csrc/runtime/env_hash.cc Outdated
Copy link
Copy Markdown
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.

mostly lgtm, minor nits

}
xla_flags.push_back(current_flag);
}
// Ensure the flags are sorted so the input order doesn't impact the hash.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh, didn't think of this. Nice catch!

Comment thread torch_xla/csrc/runtime/env_hash.h Outdated
}

torch::lazy::hash_t PjRtComputationClient::HashCompilationEnv() {
// TODO(jonbolin): Incorporate CompileOptions into the hash. These are
Copy link
Copy Markdown
Contributor

@yeounoh yeounoh Dec 4, 2023

Choose a reason for hiding this comment

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

Once we do, we don't need the SPMD mode be part of the hash (virtual device on/off).

Copy link
Copy Markdown
Contributor

@yeounoh yeounoh left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

Comment thread torch_xla/csrc/xla_graph_executor.cpp
@jonb377 jonb377 force-pushed the jonbolin/comp-env-hash branch 2 times, most recently from eeee59e to d2190e4 Compare December 5, 2023 20:56
Base automatically changed from jonbolin/persistent-cache to master December 5, 2023 22:42
@jonb377 jonb377 force-pushed the jonbolin/comp-env-hash branch from d2190e4 to aee73ba Compare December 6, 2023 01:37
@jonb377 jonb377 merged commit 1ea2374 into master Dec 6, 2023
@jonb377 jonb377 deleted the jonbolin/comp-env-hash branch December 6, 2023 16:47
jonb377 added a commit that referenced this pull request Dec 7, 2023
* Incorporate compilation environment in hash

* Add unit test

* Incorporate SPMD mode in the hash

* Always rehash in env_hash
ManfeiBai pushed a commit to ManfeiBai/PyTorchXLA that referenced this pull request Dec 8, 2023
* Incorporate compilation environment in hash

* Add unit test

* Incorporate SPMD mode in the hash

* Always rehash in env_hash
chunnienc pushed a commit to chunnienc/xla that referenced this pull request Dec 14, 2023
* Incorporate compilation environment in hash

* Add unit test

* Incorporate SPMD mode in the hash

* Always rehash in env_hash
golechwierowicz pushed a commit that referenced this pull request Jan 12, 2024
* Incorporate compilation environment in hash

* Add unit test

* Incorporate SPMD mode in the hash

* Always rehash in env_hash
bhavya01 pushed a commit that referenced this pull request Apr 22, 2024
* Incorporate compilation environment in hash

* Add unit test

* Incorporate SPMD mode in the hash

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants