Skip to content

Conversation

@awaelchli
Copy link
Contributor

@awaelchli awaelchli commented Sep 6, 2021

What does this PR do?

Fixes #9346

Reproducible only on the 1.4.x branch.
On master, the issue is already fixed by #8587. However, since that change was not a bugfix and will not be merged back to 1.4.x, I'm submitting this hotfix here as a standalone PR.

The warning appears because "args" is in the signature of the method and this matches the deprecated ones.

This should go before the 1.4.6 patch release.

Repro:

import os
from datetime import timedelta

import torch
from torch.utils.data import DataLoader, Dataset

from pytorch_lightning import LightningModule, Trainer


class RandomDataset(Dataset):
    def __init__(self, size, length):
        self.len = length
        self.data = torch.randn(length, size)

    def __getitem__(self, index):
        return self.data[index]

    def __len__(self):
        return self.len


class BoringModel(LightningModule):
    def __init__(self):
        super().__init__()
        self.layer = torch.nn.Linear(32, 2)

    def forward(self, x):
        return self.layer(x)

    def training_step(self, batch, batch_idx):
        loss = self(batch).sum()
        self.log("train_loss", loss)
        return {"loss": loss}

    def configure_optimizers(self):
        return torch.optim.SGD(self.layer.parameters(), lr=0.1)


def run():
    train_data = DataLoader(RandomDataset(32, 64), batch_size=2)

    model = BoringModel()
    trainer = Trainer(
        max_time=timedelta(seconds=10),
        default_root_dir=os.getcwd(),
        limit_train_batches=1,
        limit_val_batches=1,
        num_sanity_val_steps=0,
        max_epochs=1,
        weights_summary=None,
    )
    trainer.fit(model, train_dataloaders=train_data)


if __name__ == "__main__":
    run()

Does your PR introduce any breaking changes? If yes, please list them.

Before submitting

  • Was this discussed/approved via a GitHub issue? (not for typos and docs)
  • 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?
  • Did you make sure to update the documentation with your changes? (if necessary)
  • Did you write any new necessary tests? (not for typos and docs)
  • Did you verify new and existing tests pass locally with your changes?
  • Did you list all the breaking changes introduced by this pull request?
  • Did you update the CHANGELOG? (not for typos, docs, test updates, or internal minor changes/refactorings)

PR review

Anyone in the community is welcome to review the PR.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:

  • Is this pull request ready for review? (if not, please submit in draft mode)
  • Check that all items from Before submitting are resolved
  • 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

Did you have fun?

Make sure you had fun coding 🙃

@awaelchli awaelchli changed the title fix signature in timer to prevent deprecation warning fix signature in callbacks to prevent deprecation warning Sep 6, 2021
@awaelchli awaelchli added bug Something isn't working callback deprecation Includes a deprecation labels Sep 6, 2021
@awaelchli awaelchli added this to the v1.4.x milestone Sep 6, 2021
@awaelchli awaelchli linked an issue Sep 6, 2021 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Sep 6, 2021

Codecov Report

Merging #9347 (02a2a40) into release/1.4.x (645eabe) will not change coverage.
The diff coverage is 100%.

@@              Coverage Diff              @@
##           release/1.4.x   #9347   +/-   ##
=============================================
  Coverage             92%     92%           
=============================================
  Files                218     218           
  Lines              14490   14490           
=============================================
  Hits               13393   13393           
  Misses              1097    1097           

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 !

@mergify mergify bot added the ready PRs ready to be merged label Sep 6, 2021
@lexierule lexierule merged commit a61cc72 into release/1.4.x Sep 7, 2021
@lexierule lexierule deleted the bugfix/timer-on-train-end branch September 7, 2021 16:16
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 deprecation Includes a deprecation ready PRs ready to be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Setting max_time in trainer triggers deprecation warning

6 participants