Skip to content

feat: update lightning example to lightning 2.0#603

Open
dwahdany wants to merge 3 commits into
meta-pytorch:mainfrom
dwahdany:main
Open

feat: update lightning example to lightning 2.0#603
dwahdany wants to merge 3 commits into
meta-pytorch:mainfrom
dwahdany:main

Conversation

@dwahdany

@dwahdany dwahdany commented Sep 11, 2023

Copy link
Copy Markdown
Contributor

Types of changes

  • 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 change)
  • Docs change / refactoring / dependency upgrade

Motivation and Context / Related issue

Updates the lightning example to use a less hacky way to change datamodules.
Lightning-AI/pytorch-lightning#10430

How Has This Been Tested (if it applies)

Checklist

  • The documentation is up-to-date with the changes I made.
  • I have read the CONTRIBUTING document and completed the CLA (see CONTRIBUTING).
  • All tests passed, and additional code has been covered with new tests.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 11, 2023
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@karthikprasad karthikprasad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the PR! There is a minor bug in the variable name. I'm surprised the integration test passed despite this though.

Comment thread examples/mnist_lightning.py Outdated
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@lsc64 has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@dwahdany

dwahdany commented Oct 5, 2023

Copy link
Copy Markdown
Contributor Author

Thanks for the PR! There is a minor bug in the variable name. I'm surprised the integration test passed despite this though.

One could think it’s possible to add two lines without a major oversight, but here we are… thanks, I’ve changed it!

@dwahdany dwahdany requested a review from karthikprasad October 5, 2023 22:03
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@lsc64 has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@lsc64 has updated the pull request. You must reimport the pull request before landing.

@dwahdany

dwahdany commented Jan 5, 2024

Copy link
Copy Markdown
Contributor Author

friendly ping @karthikprasad
I think your review is implemented. let me know if something else is missing

Dariush Wahdany and others added 3 commits March 22, 2024 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants