Skip to content
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

Trace Transformation Tutorial - CPU Offloading #966

Merged
merged 15 commits into from
Aug 21, 2024

Conversation

kshitij12345
Copy link
Collaborator

@kshitij12345 kshitij12345 commented Aug 13, 2024

This PR adds a notebook demonstrating writing a transform - in this case a post optimization transform for CPU Offloading

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Aug 15, 2024
@kshitij12345 kshitij12345 changed the title [WIP] Trace Transformation Tutorial - CPU Offloading Trace Transformation Tutorial - CPU Offloading Aug 15, 2024
@kshitij12345 kshitij12345 marked this pull request as ready for review August 15, 2024 14:39
@mruberry mruberry requested a review from ptrblck August 15, 2024 14:41
Copy link
Collaborator

@riccardofelluga riccardofelluga left a comment

Choose a reason for hiding this comment

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

I'm only a little confused by the helper functions but looks good otherwise!

notebooks/writing_a_trace_transform_cpu_offloading.ipynb Outdated Show resolved Hide resolved
@kshitij12345 kshitij12345 requested a review from Fuzzkatt August 16, 2024 10:53
Copy link
Contributor

@Fuzzkatt Fuzzkatt left a comment

Choose a reason for hiding this comment

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

Thanks for the notebook, it looks really good! Left some minor discussion point questions and clarification / documentation questions, but the code looks good!

Copy link
Collaborator

@t-vi t-vi left a comment

Choose a reason for hiding this comment

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

Love it! Thank you @kshitij12345 @riccardofelluga @crcrpar @Fuzzkatt

One thing I wonder (and I know we're not doing it right in the thunder code either): I think we should require transforms to copy the trace rather than modifying it inplace.
WDYT?

@t-vi t-vi enabled auto-merge (squash) August 21, 2024 13:13
@t-vi t-vi merged commit eed91fb into Lightning-AI:main Aug 21, 2024
37 checks passed
@kshitij12345 kshitij12345 deleted the trace-transform-tutorial branch August 21, 2024 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants