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

Weekly Patch Release v.1.2.7 [full merge, no squash] #6850

Merged
merged 18 commits into from
Apr 7, 2021

Conversation

kaushikb11
Copy link
Contributor

@kaushikb11 kaushikb11 commented Apr 6, 2021

PR review

related to #6083

  • Is this pull request ready for review? (if not, please submit in draft mode)
  • 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

Borda and others added 14 commits April 6, 2021 16:12
* fix_hydra

* update changelog

Co-authored-by: Your Name <[email protected]>
* Update Bolts link

* Update Bolts link

* formt

Co-authored-by: Jirka Borovec <[email protected]>
* Update logic for checking TPUs availability

* fix flake8

* add fix
…-AI#6828)

* fix iterable dataset len check

* update predict and validate

* add validate to test

* add changelog

* add predict
* sanitize none params during pruning

* amend
* Fix DPP + SyncBN

Ensure that model is already on correct GPU before applying SyncBN conversion

* Fix order of SyncBN for ddp_spawn
@pep8speaks
Copy link

pep8speaks commented Apr 6, 2021

Hello @kaushikb11! Thanks for updating this PR.

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-04-06 19:38:02 UTC

ethanwharris and others added 2 commits April 6, 2021 17:28
)

* Add test for symlink support and initial fix

* Respond to comment and add docstring

* Update CHANGELOG.md

* Simplify

* Update pytorch_lightning/utilities/cloud_io.py

Co-authored-by: Carlos Mocholí <[email protected]>

* Make `LightningLocalFileSystem` protected

Co-authored-by: Carlos Mocholí <[email protected]>
There seem to be 3 arguments missing in the `lr_find` call in the tunining.py file.
@carmocca carmocca mentioned this pull request Apr 6, 2021
Copy link
Member

@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.

lgtm, just fix tests...

Copy link
Member

@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.

just till tests are fixed 🐰

@codecov
Copy link

codecov bot commented Apr 6, 2021

Codecov Report

Merging #6850 (caa2275) into release/1.2.x (e7abd8e) will decrease coverage by 4%.
The diff coverage is 77%.

❗ Current head caa2275 differs from pull request most recent head 86f6d08. Consider uploading reports for the commit 86f6d08 to get more accurate results

@@              Coverage Diff               @@
##           release/1.2.x   #6850    +/-   ##
==============================================
- Coverage             91%     88%    -4%     
==============================================
  Files                190     190            
  Lines              13050   13008    -42     
==============================================
- Hits               11923   11406   -517     
- Misses              1127    1602   +475     

@kaushikb11
Copy link
Contributor Author

@Borda There you go! 🎉

Skip advanced profiler for torch > 1.8

Skip pytorch profiler for torch > 1.8

Fix save checkpoint logic for TPUs
Borda
Borda previously approved these changes Apr 6, 2021
@@ -203,12 +203,14 @@ def test_step(self, *args, **kwargs):
def predict(self, *args, **kwargs):
return self.lightning_module.predict(*args, **kwargs)

def save_checkpoint(self, checkpoint: Dict[str, Any], filepath: str) -> None:
def save_checkpoint(self, filepath: str, weights_only: bool = False) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

@tchaton @kaushikb11 where/when we changed this API?

Copy link
Contributor

Choose a reason for hiding this comment

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

Changed in this PR: 646cf2f
You can use git blame to find the associated PR.

Copy link
Member

Choose a reason for hiding this comment

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

yes I know I just say that the API is different / not back compatible

@Borda Borda self-requested a review April 6, 2021 20:22
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 ! Great job.

@tchaton tchaton enabled auto-merge (squash) April 6, 2021 20:26
@kaushikb11 kaushikb11 disabled auto-merge April 7, 2021 14:57
@lexierule lexierule merged commit b2c7345 into Lightning-AI:release/1.2.x Apr 7, 2021
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.