Skip to content

Conversation

@carmocca
Copy link
Contributor

@carmocca carmocca commented Jul 27, 2022

What does this PR do?

See https://github.com/pytorch/torchdistx

Fixes #13796
Fixes #13660
Closes #13763

Does your PR introduce any breaking changes? If yes, please list them.

Removes the experimental code for meta tensor materialization.

Before submitting

  • Was this discussed/approved via a GitHub issue? (not for typos and docs)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • [n/a] Did you make sure to update the documentation with your changes? (if necessary)
  • Did you write any new necessary tests? (not for typos and docs)
  • Did you verify new and existing tests pass locally with your changes?
  • Did you list all the breaking changes introduced by this pull request?
  • Did you update the CHANGELOG? (not for typos, docs, test updates, or minor internal changes/refactors)

PR review

  • Is this pull request ready for review? (if not, please submit in draft mode)
  • Check that all items from Before submitting are resolved
  • Make sure the title is self-explanatory and the description concisely explains the PR
  • Add labels and milestones (and optionally projects) to the PR so it can be classified

cc @Borda @justusschock @awaelchli @rohitgr7 @tchaton @akihironitta

@carmocca carmocca self-assigned this Jul 27, 2022
@github-actions github-actions bot added the pl Generic label for PyTorch Lightning package label Jul 27, 2022
@carmocca carmocca added this to the pl:1.8 milestone Jul 27, 2022
@carmocca carmocca marked this pull request as ready for review July 27, 2022 09:38
@mergify mergify bot added has conflicts and removed ready PRs ready to be merged labels Jul 28, 2022
@mergify mergify bot added ready PRs ready to be merged and removed has conflicts ready PRs ready to be merged labels Jul 28, 2022
@carmocca
Copy link
Contributor Author

Converting to draft while I address the dependency issues

@carmocca carmocca marked this pull request as draft July 28, 2022 15:34
@carmocca carmocca mentioned this pull request Jul 28, 2022
10 tasks
@carmocca
Copy link
Contributor Author

carmocca commented Aug 5, 2022

Since this is all very experimental, I decided not to add the dependency. It's very tricky choosing the ABI version dynamically. And it's only released with PyTorch 1.12 compatibility.

@carmocca carmocca marked this pull request as ready for review August 5, 2022 09:46
@carmocca carmocca added deprecation Includes a deprecation and removed tests labels Aug 5, 2022
@codecov
Copy link

codecov bot commented Aug 5, 2022

Codecov Report

Merging #13868 (8fc8900) into master (0883971) will decrease coverage by 3%.
The diff coverage is 83%.

@@            Coverage Diff             @@
##           master   #13868      +/-   ##
==========================================
- Coverage      80%      76%      -3%     
==========================================
  Files         103      323     +220     
  Lines        6858    26356   +19498     
==========================================
+ Hits         5462    20116   +14654     
- Misses       1396     6240    +4844     

@carmocca carmocca enabled auto-merge (squash) August 5, 2022 11:10
Copy link
Contributor

@tchaton tchaton left a comment

Choose a reason for hiding this comment

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

LGTM !

@carmocca carmocca merged commit 91dd6a6 into master Aug 5, 2022
@carmocca carmocca deleted the refactor/torchdistx branch August 5, 2022 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deprecation Includes a deprecation experimental pl Generic label for PyTorch Lightning package ready PRs ready to be merged refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

init_meta function return type mismatch Meta tensors have side effects

5 participants