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

Fix ModelCheckpoint.CHECKPOINT_NAME_LAST test interaction #18993

Merged
merged 1 commit into from
Nov 12, 2023

Conversation

awaelchli
Copy link
Contributor

@awaelchli awaelchli commented Nov 12, 2023

What does this PR do?

Fixes a test interaction seen in the master branch. The class attribute is set in one test, and then reset at the end of the test. In case this test fails for some reason, the patched attribute ModelCheckpoint.CHECKPOINT_NAME_LAST would carry over into other tests, causing them to fail since the checkpoint name assertion expects a different "last" name.


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

cc @carmocca @awaelchli @Borda

@awaelchli awaelchli added bug Something isn't working tests labels Nov 12, 2023
@github-actions github-actions bot added pl Generic label for PyTorch Lightning package and removed bug Something isn't working tests labels Nov 12, 2023
@awaelchli awaelchli added this to the 2.1.x milestone Nov 12, 2023
Copy link
Contributor

github-actions bot commented Nov 12, 2023

⚡ Required checks status: All passing 🟢

Groups summary

🟢 pytorch_lightning: Tests workflow
Check ID Status
pl-cpu (macOS-11, lightning, 3.8, 1.12, oldest) success
pl-cpu (macOS-11, lightning, 3.9, 1.12) success
pl-cpu (macOS-11, lightning, 3.10, 1.13) success
pl-cpu (macOS-11, lightning, 3.10, 2.0) success
pl-cpu (macOS-11, lightning, 3.10, 2.1) success
pl-cpu (ubuntu-20.04, lightning, 3.8, 1.12, oldest) success
pl-cpu (ubuntu-20.04, lightning, 3.9, 1.12) success
pl-cpu (ubuntu-20.04, lightning, 3.10, 1.13) success
pl-cpu (ubuntu-20.04, lightning, 3.10, 2.0) success
pl-cpu (ubuntu-20.04, lightning, 3.10, 2.1) success
pl-cpu (windows-2022, lightning, 3.8, 1.12, oldest) success
pl-cpu (windows-2022, lightning, 3.9, 1.12) success
pl-cpu (windows-2022, lightning, 3.10, 1.13) success
pl-cpu (windows-2022, lightning, 3.10, 2.0) success
pl-cpu (windows-2022, lightning, 3.10, 2.1) success
pl-cpu (macOS-11, pytorch, 3.8, 1.13) success
pl-cpu (ubuntu-20.04, pytorch, 3.8, 1.13) success
pl-cpu (windows-2022, pytorch, 3.8, 1.13) success
pl-cpu (macOS-12, pytorch, 3.11, 2.0) success
pl-cpu (macOS-12, pytorch, 3.11, 2.1) success
pl-cpu (ubuntu-22.04, pytorch, 3.11, 2.0) success
pl-cpu (ubuntu-22.04, pytorch, 3.11, 2.1) success
pl-cpu (windows-2022, pytorch, 3.11, 2.0) success
pl-cpu (windows-2022, pytorch, 3.11, 2.1) success

These checks are required after the changes to tests/tests_pytorch/checkpointing/test_model_checkpoint.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/checkpointing/test_model_checkpoint.py.


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.

@awaelchli awaelchli added tests bug Something isn't working labels Nov 12, 2023
Copy link

codecov bot commented Nov 12, 2023

Codecov Report

Merging #18993 (275e4df) into master (466f772) will decrease coverage by 27%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #18993      +/-   ##
==========================================
- Coverage      76%      48%     -27%     
==========================================
  Files         450      442       -8     
  Lines       36460    36307     -153     
==========================================
- Hits        27563    17507   -10056     
- Misses       8897    18800    +9903     

@carmocca carmocca merged commit b4605b4 into master Nov 12, 2023
@carmocca carmocca deleted the tests/set-last branch November 12, 2023 10:01
@mergify mergify bot added the ready PRs ready to be merged label Nov 12, 2023
@awaelchli awaelchli added the fun Staff contributions outside working hours - to differentiate from the "community" label label Nov 12, 2023
Borda pushed a commit that referenced this pull request Nov 14, 2023
lantiga pushed a commit that referenced this pull request Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working callback: model checkpoint fun Staff contributions outside working hours - to differentiate from the "community" label pl Generic label for PyTorch Lightning package ready PRs ready to be merged tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants