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

Refactor autocast to AutocastTransform for Improved Composability and Transform Integration #1516

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

rittik9
Copy link

@rittik9 rittik9 commented Dec 4, 2024

Before submitting
  • Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure to update the docs?
  • Did you write any new necessary tests?

What does this PR do?

Fixes #1122

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

@rittik9 rittik9 marked this pull request as draft December 4, 2024 10:31
@t-vi
Copy link
Collaborator

t-vi commented Dec 4, 2024

Hi @rittik9 , thank you for working on this!

So two quick comments and let me know how much you want to go into details or explore yourself:

  • this should work without changing thunder/__init__.py, quite likely, the best thing is to define the transform_traces_pre_prologue method on the transform and do the changing there (see eg quantization for an example),
  • it would be cool if we could move from eval_trace to using the brand-new thunder.trace_interpreter.TraceSubstitutionProcessor.

@rittik9
Copy link
Author

rittik9 commented Dec 4, 2024

@t-vi Thank you for your suggestions! I’m still getting familiar with the codebase, so it might take me some time. I’ll definitely review the examples you mentioned and work on incorporating your suggestions in this PR.

@rittik9 rittik9 force-pushed the rittik/autocast branch 5 times, most recently from f88ef7a to 2f51b44 Compare December 9, 2024 16:55
@rittik9 rittik9 marked this pull request as ready for review December 11, 2024 12:34
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.

Make autocast a Transform
2 participants