Add export and Dynamo capture fallback - #20
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
aryan5v
marked this pull request as ready for review
August 1, 2026 04:18
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack
This PR is intentionally stacked on
fx-capture-hooks/ PR #19. It should be reviewed and merged after that foundation branch.What changed
auto: symbolic FX, non-stricttorch.export, then Dynamo.ModuleHookManageroriginals, compiler-disabled attention forwards, and observed forward context only inside capture.Root cause
torch.exportcannot inspect FastVideo'sfunctools.partialoffload wrapper, and Dynamo intentionally skips compiler-disabled offload/attention entry points. The first bypass also called the offload pre-hook, which prefetches the next layer and polluted the next normal generation. Capture now loads only the current module state and restores the complete prior state observationally.Validation
No weights, prompts, activations, tensor values, source, or credentials are serialized.
Greptile Summary
This PR adds Dynamo as a third capture backend, implements a symbolic→export→dynamo fallback chain in
automode, and introduces three context managers that temporarily bypass FastVideo's offload wrapper, re-establish the observed forward context, and expose compiler-disabled attention forwards — all with complete state restoration infinallyblocks regardless of trace outcome._regions_for): each shape variant is tried against all modes in order; per-variant tensor references (example_args,example_kwargs,observed_context) are cleared in a guaranteedfinally, with a second outerfinallyinfinalize()covering variants that were never reached due to an early exception._capture_ready_module): saves parameter data andgpu_named_parameters, callswait_and_replace_params()eagerly, exposesoriginal_forward, then restores the exact wrapped state; rejects unknownModuleHookManagerhooks to prevent silent semantic changes.capture_mode,capture_attempts,capture_failures, andcapture_mode_breakdowngive downstream consumers full visibility into how each region was captured.Confidence Score: 4/5
Safe to merge after the schema version question is resolved; all new context managers restore state correctly and every tensor reference is guaranteed to be cleared before finalization returns.
The fallback logic, state-restoration guarantees, and test coverage are thorough. The one open question is whether forward_batch=None in the reproduced forward context can cause a different control-flow branch to be captured for production attention modules — acceptable for metadata profiling but worth a conscious decision.
Files Needing Attention: fastvideo/optimization/fx_capture.py — specifically _capture_forward_context (forward_batch omission) and _trace (dynamo return-type handling).
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[_regions_for: iterate variants] --> B[_mode_order returns attempt list] B -->|auto| C[symbolic, export, dynamo] B -->|explicit mode| D[single mode] B -->|invalid| E[ValueError - finally clears variant refs] C --> F[_trace: symbolic] D --> F F -->|success| K[extract_graph check operations] F -->|failure| G[record failure - next mode] G --> H[_trace: export via _capture_ready_module + _traceable_module_forwards + _capture_forward_context] H -->|success| K H -->|failure| I[_trace: dynamo] I -->|success| K I -->|all failed| J[capture_mode=None - continue] K -->|empty ops| J K -->|ops present| L[capture_mode set - build region dict] L --> M[finally: clear example_args/kwargs/context] J --> M E --> N[finalize finally: clear all remaining variants] M --> O[append region to results]Reviews (2): Last reviewed commit: "fix: reject undocumented FX tracer alias..." | Re-trigger Greptile