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

Support deprecated attribute usage #699

Merged
merged 5 commits into from
Aug 9, 2021
Merged

Support deprecated attribute usage #699

merged 5 commits into from
Aug 9, 2021

Conversation

carmocca
Copy link
Contributor

@carmocca carmocca commented Aug 4, 2021

What does this PR do?

Reported in Slack by Tran N.M. Hoang

Hi, I have met a bug when using lightning v1.4 and lightning-bolts to train Moco_v2, it seems that since lightning v1.4, self.trainer.use_ddp2 and self.trainer.use_ddp are removed but Moco_v2 implemented by lightning-bolts stills uses this functionality, so I cannot train Moco_v2 anymore. Can you check it?

Before submitting

  • Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
  • 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?
  • Did you write any new necessary tests? [not needed for typos/docs]
  • Did you verify new and existing tests pass locally with your changes?
  • [n/a] If you made a notable change (that affects users), did you update the CHANGELOG?

PR review

  • Is this pull request ready for review? (if not, please submit in draft mode)

@carmocca carmocca added the fix fixing issues... label Aug 4, 2021
@carmocca carmocca self-assigned this Aug 4, 2021
@github-actions github-actions bot added the model label Aug 4, 2021
@codecov
Copy link

codecov bot commented Aug 4, 2021

Codecov Report

Merging #699 (82d080e) into master (73f2f99) will decrease coverage by 0.43%.
The diff coverage is 89.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #699      +/-   ##
==========================================
- Coverage   72.03%   71.60%   -0.44%     
==========================================
  Files         119      119              
  Lines        7356     7367      +11     
==========================================
- Hits         5299     5275      -24     
- Misses       2057     2092      +35     
Flag Coverage Δ
cpu 71.60% <89.47%> (-0.44%) ⬇️
pytest 71.60% <89.47%> (-0.44%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pl_bolts/models/rl/dqn_model.py 80.44% <85.71%> (-0.02%) ⬇️
..._bolts/models/self_supervised/moco/moco2_module.py 59.42% <88.88%> (+0.60%) ⬆️
pl_bolts/models/rl/double_dqn_model.py 95.83% <100.00%> (ø)
pl_bolts/models/rl/per_dqn_model.py 86.88% <100.00%> (ø)
pl_bolts/utils/__init__.py 92.85% <100.00%> (+0.26%) ⬆️
pl_bolts/datasets/cifar10_dataset.py 72.04% <0.00%> (-25.81%) ⬇️
pl_bolts/datasets/base_dataset.py 81.39% <0.00%> (-13.96%) ⬇️
...l_bolts/models/rl/vanilla_policy_gradient_model.py 93.44% <0.00%> (-2.46%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 73f2f99...82d080e. Read the comment docs.

pl_bolts/models/rl/dqn_model.py Outdated Show resolved Hide resolved
pl_bolts/models/self_supervised/moco/moco2_module.py Outdated Show resolved Hide resolved
@Borda Borda merged commit 5ab4fae into master Aug 9, 2021
@Borda Borda deleted the bugfix/bc-use-ddp branch August 9, 2021 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix fixing issues... model
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants