Skip to content

Commit

Permalink
LinghtningCLI now will not allow setting a class instance as a default (
Browse files Browse the repository at this point in the history
#18822)

(cherry picked from commit c5a731c)
  • Loading branch information
mauvilsa authored and lantiga committed Dec 20, 2023
1 parent d5b9951 commit af2f3f8
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 4 deletions.
2 changes: 1 addition & 1 deletion requirements/pytorch/extra.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
matplotlib>3.1, <3.9.0
omegaconf >=2.0.5, <2.4.0
hydra-core >=1.0.5, <1.4.0
jsonargparse[signatures] >=4.18.0, <4.28.0
jsonargparse[signatures] >=4.26.1, <4.28.0
rich >=12.3.0, <13.6.0
tensorboardX >=2.2, <2.7.0 # min version is set by torch.onnx missing attribute
bitsandbytes <=0.41.1
4 changes: 4 additions & 0 deletions src/lightning/pytorch/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).

## [2.1.3] - 2023-12-21

### Changed

- `LightningCLI` no longer allows setting a normal class instance as default. A `lazy_instance` can be used instead ([#18822](https://github.com/Lightning-AI/lightning/pull/18822))

### Fixed

- Fixed checks for local file protocol due to fsspec changes in 2023.10.0 ([#19023](https://github.com/Lightning-AI/lightning/pull/19023))
Expand Down
2 changes: 1 addition & 1 deletion src/lightning/pytorch/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
from lightning.pytorch.utilities.model_helpers import is_overridden
from lightning.pytorch.utilities.rank_zero import rank_zero_warn

_JSONARGPARSE_SIGNATURES_AVAILABLE = RequirementCache("jsonargparse[signatures]>=4.18.0")
_JSONARGPARSE_SIGNATURES_AVAILABLE = RequirementCache("jsonargparse[signatures]>=4.26.1")

if _JSONARGPARSE_SIGNATURES_AVAILABLE:
import docstring_parser
Expand Down
9 changes: 7 additions & 2 deletions tests/tests_pytorch/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@
else:
from argparse import Namespace

def lazy_instance(*args, **kwargs):
return None


@contextmanager
def mock_subclasses(baseclass, *subclasses):
Expand Down Expand Up @@ -173,7 +176,9 @@ def on_fit_start(self):
self.trainer.ran_asserts = True

with mock.patch("sys.argv", ["any.py", "fit", f"--trainer.callbacks={json.dumps(callbacks)}"]):
cli = LightningCLI(TestModel, trainer_defaults={"fast_dev_run": True, "logger": CSVLogger(".")})
cli = LightningCLI(
TestModel, trainer_defaults={"fast_dev_run": True, "logger": lazy_instance(CSVLogger, save_dir=".")}
)

assert cli.trainer.ran_asserts

Expand Down Expand Up @@ -589,7 +594,7 @@ def on_fit_start(self):

# mps not yet supported by distributed
@RunIf(skip_windows=True, mps=False)
@pytest.mark.parametrize("logger", [False, TensorBoardLogger(".")])
@pytest.mark.parametrize("logger", [False, lazy_instance(TensorBoardLogger, save_dir=".")])
@pytest.mark.parametrize("strategy", ["ddp_spawn", "ddp"])
def test_cli_distributed_save_config_callback(cleandir, logger, strategy):
from torch.multiprocessing import ProcessRaisedException
Expand Down

0 comments on commit af2f3f8

Please sign in to comment.