-
Notifications
You must be signed in to change notification settings - Fork 13
fix: apply dynamo config on GPU worker thread (#167) #178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,22 @@ | ||
| import torch | ||
|
|
||
| torch._dynamo.config.recompile_limit = 84 | ||
| torch._dynamo.config.allow_unspec_int_on_nn_module = True | ||
| torch._dynamo.config.specialize_int = False | ||
| torch.set_float32_matmul_precision('high') | ||
| # torch._dynamo ConfigModule stores user overrides in a ContextVar (thread-local | ||
| # as of torch 2.13+). Import-time assignments only affect the importing thread; | ||
| # the dedicated GPU executor thread that actually compiles never sees them. | ||
| # Call apply_dynamo_config() on any thread that may trigger dynamo tracing. | ||
| RECOMPILE_LIMIT = 84 | ||
|
|
||
|
|
||
| def apply_dynamo_config() -> None: | ||
| """Apply dynamo settings to the *calling* thread. | ||
|
|
||
| torch's ConfigModule keeps user overrides in a ContextVar, so these are | ||
| thread-local — any thread that may trigger a compile must call this. | ||
| """ | ||
| torch._dynamo.config.recompile_limit = RECOMPILE_LIMIT | ||
| torch._dynamo.config.allow_unspec_int_on_nn_module = True | ||
| torch._dynamo.config.specialize_int = False | ||
|
|
||
|
|
||
| apply_dynamo_config() | ||
| torch.set_float32_matmul_precision("high") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Although |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -609,11 +609,14 @@ def _capture_slots( | |
| # graph (finished in the submodule ``postprocess``). | ||
| forward = getattr(submodule, config.capture_forward_method) | ||
| if config.compile: | ||
| from mstar.engine import RECOMPILE_LIMIT | ||
|
|
||
| forward = torch.compile( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Heads up: a bare With Suggest deriving it instead: |
||
| forward, | ||
| mode="max-autotune-no-cudagraphs", | ||
| fullgraph=False, | ||
| dynamic=False, | ||
| recompile_limit=RECOMPILE_LIMIT, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, I realize that my suggested fix in #167 of setting With respect to my comment https://github.com/mstar-project/mstar/pull/178/changes#r3612126831 about computing the recompile limit based on the cuda graph configurations, I think the best way to proceed is to put
|
||
| ) | ||
| spec.engine_inputs.sampler.applied_penalty_in_graph = False | ||
|
|
||
|
|
@@ -2184,11 +2187,14 @@ def _capture_one( | |
|
|
||
| fwd = submodule.forward_batched | ||
| if config.compile: | ||
| from mstar.engine import RECOMPILE_LIMIT | ||
|
|
||
| fwd = torch.compile( | ||
| fwd, | ||
| mode="max-autotune-no-cudagraphs", | ||
| fullgraph=False, | ||
| dynamic=False, | ||
| recompile_limit=RECOMPILE_LIMIT, | ||
| ) | ||
|
|
||
| # Warmup and capture on the shared side stream (see warmup_and_capture | ||
|
|
@@ -2509,11 +2515,14 @@ def _capture_one(self, shape: PiecewiseCaptureShape) -> None: | |
|
|
||
| fn = self.config.capture_fn | ||
| if self.config.compile: | ||
| from mstar.engine import RECOMPILE_LIMIT | ||
|
|
||
| fn = torch.compile( | ||
| fn, | ||
| mode="max-autotune-no-cudagraphs", | ||
| fullgraph=False, | ||
| dynamic=False, | ||
| recompile_limit=RECOMPILE_LIMIT, | ||
| ) | ||
|
|
||
| def run_fn(): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| """Regression: dynamo config must apply on worker threads (#167). | ||
|
|
||
| torch._dynamo ConfigModule stores user overrides in a ContextVar (thread-local | ||
| as of torch 2.13+). Import-time assignments on the main thread do not propagate | ||
| to the dedicated GPU executor thread. apply_dynamo_config() must be called on | ||
| any thread that may trigger dynamo tracing. | ||
| """ | ||
|
|
||
| from concurrent.futures import ThreadPoolExecutor | ||
|
|
||
| import torch | ||
|
|
||
| from mstar.engine import RECOMPILE_LIMIT, apply_dynamo_config | ||
|
|
||
| _KEYS = ("recompile_limit", "allow_unspec_int_on_nn_module", "specialize_int") | ||
|
|
||
|
|
||
| def _read_dynamo_flags() -> dict: | ||
| return {k: getattr(torch._dynamo.config, k) for k in _KEYS} | ||
|
|
||
|
|
||
| def test_apply_dynamo_config_on_worker_thread(): | ||
| """GPU-thread initializer path: flags match RECOMPILE_LIMIT / expected defaults.""" | ||
| # Main thread already applied at import; re-apply for a clean baseline. | ||
| apply_dynamo_config() | ||
| main = _read_dynamo_flags() | ||
| assert main["recompile_limit"] == RECOMPILE_LIMIT | ||
| assert main["allow_unspec_int_on_nn_module"] is True | ||
| assert main["specialize_int"] is False | ||
|
|
||
| with ThreadPoolExecutor(max_workers=1, initializer=apply_dynamo_config) as ex: | ||
| worker = ex.submit(_read_dynamo_flags).result() | ||
|
|
||
| assert worker["recompile_limit"] == RECOMPILE_LIMIT | ||
| assert worker["allow_unspec_int_on_nn_module"] is True | ||
| assert worker["specialize_int"] is False | ||
|
|
||
|
|
||
| def test_import_time_flags_do_not_leak_to_fresh_thread_without_initializer(): | ||
| """Document the torch 2.13+ ContextVar pitfall the fix addresses. | ||
|
|
||
| On builds where ConfigModule is thread-local, a fresh thread without | ||
| apply_dynamo_config() does not see main-thread overrides. Skip when the | ||
| installed torch still shares config across threads (pre-2.13 behavior). | ||
| """ | ||
| apply_dynamo_config() | ||
| main = _read_dynamo_flags() | ||
|
|
||
| with ThreadPoolExecutor(max_workers=1) as ex: | ||
| worker = ex.submit(_read_dynamo_flags).result() | ||
|
|
||
| if worker == main: | ||
| # Older torch: config is process-global — nothing to regress against. | ||
| return | ||
|
|
||
| # Thread-local: worker should have fallen back away from our overrides. | ||
| assert worker["recompile_limit"] != RECOMPILE_LIMIT or worker[ | ||
| "allow_unspec_int_on_nn_module" | ||
| ] is not True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RECOMPILE_LIMITshould be able to be overridden as an environment variable, with reasonable minimum and maximum limits set in the code (and then documented indocs/environment_variables.rst)