Skip to content

Conversation

@lantiga
Copy link
Collaborator

@lantiga lantiga commented Jan 6, 2025

What does this PR do?

Fixes #20517

Thanks to @simon-bachhuber for the fix


📚 Documentation preview 📚: https://pytorch-lightning--20528.org.readthedocs.build/en/20528/

@github-actions github-actions bot added docs Documentation related pl Generic label for PyTorch Lightning package labels Jan 6, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jan 6, 2025

⚡ Required checks status: All passing 🟢

Groups summary

🟢 pytorch_lightning: Tests workflow
Check ID Status
pl-cpu (macOS-14, lightning, 3.9, 2.1, oldest) success
pl-cpu (macOS-14, lightning, 3.10, 2.1) success
pl-cpu (macOS-14, lightning, 3.11, 2.2.2) success
pl-cpu (macOS-14, lightning, 3.11, 2.3) success
pl-cpu (macOS-14, lightning, 3.12.7, 2.4.1) success
pl-cpu (macOS-14, lightning, 3.12.7, 2.5.1) success
pl-cpu (ubuntu-20.04, lightning, 3.9, 2.1, oldest) success
pl-cpu (ubuntu-20.04, lightning, 3.10, 2.1) success
pl-cpu (ubuntu-20.04, lightning, 3.11, 2.2.2) success
pl-cpu (ubuntu-20.04, lightning, 3.11, 2.3) success
pl-cpu (ubuntu-22.04, lightning, 3.12.7, 2.4.1) success
pl-cpu (ubuntu-22.04, lightning, 3.12.7, 2.5.1) success
pl-cpu (windows-2022, lightning, 3.9, 2.1, oldest) success
pl-cpu (windows-2022, lightning, 3.10, 2.1) success
pl-cpu (windows-2022, lightning, 3.11, 2.2.2) success
pl-cpu (windows-2022, lightning, 3.11, 2.3) success
pl-cpu (windows-2022, lightning, 3.12.7, 2.4.1) success
pl-cpu (windows-2022, lightning, 3.12.7, 2.5.1) success
pl-cpu (macOS-14, pytorch, 3.9, 2.1) success
pl-cpu (ubuntu-20.04, pytorch, 3.9, 2.1) success
pl-cpu (windows-2022, pytorch, 3.9, 2.1) success
pl-cpu (macOS-14, pytorch, 3.12.7, 2.5.1) success
pl-cpu (ubuntu-22.04, pytorch, 3.12.7, 2.5.1) success
pl-cpu (windows-2022, pytorch, 3.12.7, 2.5.1) success

These checks are required after the changes to tests/tests_pytorch/helpers/advanced_models.py, tests/tests_pytorch/helpers/test_models.py.

🟢 pytorch_lightning: Azure GPU
Check ID Status
pytorch-lightning (GPUs) (testing Lightning | latest) success
pytorch-lightning (GPUs) (testing PyTorch | latest) success

These checks are required after the changes to tests/tests_pytorch/helpers/advanced_models.py, tests/tests_pytorch/helpers/test_models.py.

🟢 pytorch_lightning: Docs
Check ID Status
docs-make (pytorch, doctest) success
docs-make (pytorch, html) success

These checks are required after the changes to docs/source-pytorch/common/tbptt.rst.


Thank you for your contribution! 💜

Note
This comment is automatically generated and updates for 60 minutes every 180 seconds. If you have any other questions, contact carmocca for help.

@Borda
Copy link
Collaborator

Borda commented Jan 6, 2025

could we also add a test for it?

@t-vi
Copy link
Contributor

t-vi commented Jan 6, 2025

@Borda given that it is in the example, wouldn't it be in the doc build (or in a bit of code that cannot be run on the CI due to being too large)?

@lantiga
Copy link
Collaborator Author

lantiga commented Jan 6, 2025

Actually, I would like this to be a copy-pastable example, which this isn't. More work to be done.

@lantiga
Copy link
Collaborator Author

lantiga commented Jan 6, 2025

Alright, I made it correct and runnable.

@lantiga
Copy link
Collaborator Author

lantiga commented Jan 6, 2025

I added the corresponding test, it's very lightweight

@codecov
Copy link

codecov bot commented Jan 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79%. Comparing base (efe311c) to head (fb9e836).
Report is 45 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (efe311c) and HEAD (fb9e836). Click for more details.

HEAD has 144 uploads less than BASE
Flag BASE (efe311c) HEAD (fb9e836)
cpu 70 35
lightning 50 28
pytest 33 0
python3.9 18 9
python3.11 21 11
python3.10 9 4
lightning_fabric 11 0
python3.12.7 22 11
pytorch2.1 14 13
pytest-full 37 35
pytorch2.2.2 9 6
pytorch_lightning 9 7
Additional details and impacted files
@@            Coverage Diff            @@
##           master   #20528     +/-   ##
=========================================
- Coverage      87%      79%     -8%     
=========================================
  Files         267      264      -3     
  Lines       23380    23325     -55     
=========================================
- Hits        20235    18366   -1869     
- Misses       3145     4959   +1814     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

@Borda Borda left a comment

Choose a reason for hiding this comment

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

Looks good :)

@lantiga lantiga merged commit 76f0c54 into master Jan 6, 2025
45 checks passed
@lantiga lantiga deleted the luca/fix-tbptt-example branch January 6, 2025 17:51
Borda pushed a commit that referenced this pull request Feb 14, 2025
* Fix TBPTT example

* Make example self-contained

* Update imports

* Add test

(cherry picked from commit 76f0c54)
lexierule pushed a commit that referenced this pull request Mar 18, 2025
* Fix TBPTT example

* Make example self-contained

* Update imports

* Add test

(cherry picked from commit 76f0c54)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation related pl Generic label for PyTorch Lightning package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Documentation Example of Truncated BPTT is not working; self.optimizer.step() makes no sense

4 participants