Skip to content

[#5249][AutoDeploy] Refactor the signatures of AD graph transformations to return None - #71

Merged
galagam merged 3 commits into
feat/ad-2025-07-07from
user/ghubaraagam/issue-5249
Jul 8, 2025
Merged

[#5249][AutoDeploy] Refactor the signatures of AD graph transformations to return None#71
galagam merged 3 commits into
feat/ad-2025-07-07from
user/ghubaraagam/issue-5249

Conversation

@galagam

@galagam galagam commented Jul 3, 2025

Copy link
Copy Markdown

Description

Refactor signatures of AD graph transformations from
gm = transformation(gm)
to
transformation(gm)

Since the AD graph transformations modify the input GraphModule
in-place. Previous signature style was misleading.

See also: NVIDIA#5249

Test Coverage

Refactor-only change. Using existing unit tests to verify.

@galagam galagam self-assigned this Jul 3, 2025
@galagam
galagam marked this pull request as ready for review July 3, 2025 15:36
@galagam
galagam requested review from lucaslie and nzmora-nvidia July 3, 2025 15:37
@galagam

galagam commented Jul 3, 2025

Copy link
Copy Markdown
Author

/bot run

@nzmora-nvidia nzmora-nvidia left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM, with one fix.

Comment thread tensorrt_llm/_torch/auto_deploy/transformations/export.py
@galagam
galagam force-pushed the user/ghubaraagam/issue-5249 branch from 8e433c8 to 9ee0b3b Compare July 6, 2025 15:24

@lucaslie lucaslie left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please revert the changes in tensorrt_llm/_torch/compilation. Looks good to me otherwise! Thank you for updating this

Comment thread tensorrt_llm/_torch/compilation/backend.py Outdated
Comment thread tensorrt_llm/_torch/compilation/piecewise_optimizer.py Outdated
Comment thread tensorrt_llm/_torch/compilation/recover_pass.py Outdated
Comment thread tests/unittest/_torch/pattern_watcher.py
galagam added 3 commits July 7, 2025 09:23
…VIDIA#5249)

Refactor signatures of AD graph transformations from
  gm = transformation(gm)
to
  transformation(gm)

Since the AD graph transformations modify the input GraphModule
in-place. Previous signature style was misleading.

Signed-off-by: Gal Hubara Agam <96368689+galagam@users.noreply.github.com>
Signed-off-by: Gal Hubara Agam <96368689+galagam@users.noreply.github.com>
Signed-off-by: Gal Hubara Agam <96368689+galagam@users.noreply.github.com>
@galagam
galagam force-pushed the user/ghubaraagam/issue-5249 branch from 9ee0b3b to f06b731 Compare July 7, 2025 18:43
@galagam
galagam changed the base branch from feat/ad-2025-06-29 to feat/ad-2025-07-07 July 7, 2025 18:43
@galagam
galagam merged commit dad1c62 into feat/ad-2025-07-07 Jul 8, 2025
1 check passed
lucaslie pushed a commit that referenced this pull request Jul 18, 2025
…rmations to return None (#71)

* Refactor the signatures of AD graph transformations to return None (NVIDIA#5249)

Refactor signatures of AD graph transformations from
  gm = transformation(gm)
to
  transformation(gm)

Since the AD graph transformations modify the input GraphModule
in-place. Previous signature style was misleading.

Signed-off-by: Gal Hubara Agam <96368689+galagam@users.noreply.github.com>
lucaslie pushed a commit that referenced this pull request Jul 21, 2025
…rmations to return None (#71)

* Refactor the signatures of AD graph transformations to return None (NVIDIA#5249)

Refactor signatures of AD graph transformations from
  gm = transformation(gm)
to
  transformation(gm)

Since the AD graph transformations modify the input GraphModule
in-place. Previous signature style was misleading.

Signed-off-by: Gal Hubara Agam <96368689+galagam@users.noreply.github.com>
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.

4 participants