Skip to content

Conversation

keehyuna
Copy link
Collaborator

@keehyuna keehyuna commented Dec 2, 2024

Description

Add flag indicates to reevaluate the runtime settings when weight streaming budget is changed.
Fixes #3308

Type of change

Please delete options that are not relevant and/or add your own.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project (You can use the linters)
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas and hacks
  • I have made corresponding changes to the documentation
  • I have added tests to verify my fix or my feature
  • New and existing unit tests pass locally with my changes
  • I have added the relevant labels to my PR in so that relevant reviewers are notified

@github-actions github-actions bot added component: core Issues re: The core compiler component: api [Python] Issues re: Python API component: runtime component: dynamo Issues relating to the `torch.compile` or `torch._dynamo.export` paths labels Dec 2, 2024
@keehyuna keehyuna self-assigned this Dec 2, 2024
@github-actions github-actions bot requested a review from bowang007 December 2, 2024 11:45

// Whether cudagraphs needs to record the graph on this pass
bool need_cudagraphs_record = (CUDAGRAPHS_MODE && (!_cudagraphs_validate_shapes(inputs, compiled_engine)));
bool need_cudagraphs_record =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like we need something a bit more comprehensive than a bunch of booleans for cudagraph rerecord

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@github-actions github-actions bot added documentation Improvements or additions to documentation component: tests Issues re: Tests labels Dec 3, 2024
@keehyuna keehyuna marked this pull request as draft December 3, 2024 13:50
@keehyuna
Copy link
Collaborator Author

keehyuna commented Dec 3, 2024

Will adopt proper structure for runtime states after #3276 is reviewed.

@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Dec 18, 2024
@keehyuna keehyuna requested review from peri044 and removed request for bowang007 December 18, 2024 09:09
@keehyuna keehyuna marked this pull request as ready for review December 18, 2024 09:09
Copy link
Collaborator

@peri044 peri044 left a comment

Choose a reason for hiding this comment

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

Functionality LGTM

def test_weight_streaming_cudagraphs(self, _, use_python_runtime):
model = SampleModel().eval().cuda()
input = [torch.randn(*INPUT_SIZE, dtype=torch.float32).cuda()]
fx_graph = torch.fx.symbolic_trace(model)
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you use torch.export.export() to export the model?

name="x",
)
model = SampleModel().eval().cuda()
fx_graph = torch.fx.symbolic_trace(model)
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here. Use torch.export.export()

@peri044 peri044 merged commit 70e2a38 into pytorch:main Dec 20, 2024
68 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed component: api [Python] Issues re: Python API component: core Issues re: The core compiler component: dynamo Issues relating to the `torch.compile` or `torch._dynamo.export` paths component: runtime component: tests Issues re: Tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 [Bug] Error when weigh streaming and cuda graphs is used
4 participants