Skip to content

Commit

Permalink
Fix configuration validation error message in Lite CLI (#16334)
Browse files Browse the repository at this point in the history
  • Loading branch information
awaelchli authored and nicolai86 committed Jan 12, 2023
1 parent 93b0b78 commit e50659a
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 11 deletions.
2 changes: 2 additions & 0 deletions src/lightning_fabric/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).

- Restored sampling parity between PyTorch and Fabric dataloaders when using the `DistributedSampler` ([#16101](https://github.com/Lightning-AI/lightning/issues/16101))

- Fixes an issue where the error message wouldn't tell the user the real value that was passed through the CLI ([#16334](https://github.com/Lightning-AI/lightning/issues/16334))


## [1.8.6] - 2022-12-21

Expand Down
2 changes: 1 addition & 1 deletion src/lightning_fabric/connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ def _argument_from_env(name: str, current: Any, default: Any) -> Any:
if env_value is not None and env_value != str(current) and str(current) != str(default):
raise ValueError(
f"Your code has `Fabric({name}={current!r}, ...)` but it conflicts with the value "
f"`--{name}={current}` set through the CLI. "
f"`--{name}={env_value}` set through the CLI. "
" Remove it either from the CLI or from the Lightning Fabric object."
)
if env_value is None:
Expand Down
15 changes: 5 additions & 10 deletions tests/tests_fabric/test_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
# limitations under the License

import os
from re import escape
from typing import Any, Dict
from unittest import mock

Expand Down Expand Up @@ -808,27 +807,23 @@ def test_devices_from_environment(*_):
def test_arguments_from_environment_collision():
"""Test that the connector raises an error when the CLI settings conflict with settings in the code."""
with mock.patch.dict(os.environ, {"LT_ACCELERATOR": "cpu"}):
with pytest.raises(
ValueError, match=escape("Your code has `Fabric(accelerator='cuda', ...)` but it conflicts")
):
with pytest.raises(ValueError, match="`Fabric\\(accelerator='cuda', ...\\)` but .* `--accelerator=cpu`"):
_Connector(accelerator="cuda")

with mock.patch.dict(os.environ, {"LT_STRATEGY": "ddp"}):
with pytest.raises(
ValueError, match=escape("Your code has `Fabric(strategy='ddp_spawn', ...)` but it conflicts")
):
with pytest.raises(ValueError, match="`Fabric\\(strategy='ddp_spawn', ...\\)` but .* `--strategy=ddp`"):
_Connector(strategy="ddp_spawn")

with mock.patch.dict(os.environ, {"LT_DEVICES": "2"}):
with pytest.raises(ValueError, match=escape("Your code has `Fabric(devices=3, ...)` but it conflicts")):
with pytest.raises(ValueError, match="`Fabric\\(devices=3, ...\\)` but .* `--devices=2`"):
_Connector(devices=3)

with mock.patch.dict(os.environ, {"LT_NUM_NODES": "3"}):
with pytest.raises(ValueError, match=escape("Your code has `Fabric(num_nodes=2, ...)` but it conflicts")):
with pytest.raises(ValueError, match="`Fabric\\(num_nodes=2, ...\\)` but .* `--num_nodes=3`"):
_Connector(num_nodes=2)

with mock.patch.dict(os.environ, {"LT_PRECISION": "16"}):
with pytest.raises(ValueError, match=escape("Your code has `Fabric(precision=64, ...)` but it conflicts")):
with pytest.raises(ValueError, match="`Fabric\\(precision=64, ...\\)` but .* `--precision=16`"):
_Connector(precision=64)


Expand Down

0 comments on commit e50659a

Please sign in to comment.