[train][lightning] change remaining pytorch_lightning imports#61291
[train][lightning] change remaining pytorch_lightning imports#61291liulehui wants to merge 5 commits intoray-project:masterfrom
Conversation
Signed-off-by: Lehui Liu <lehui@anyscale.com>
There was a problem hiding this comment.
Code Review
This pull request correctly updates the PyTorch Lightning imports from pytorch_lightning to lightning.pytorch for compatibility with version 2.0 and later. The changes are well-executed across various files, including documentation, examples, and tests. I have a couple of suggestions to enhance maintainability and consistency.
| "import os\n", | ||
| "import torch\n", | ||
| "import tempfile\n", | ||
| "import lightning.pytorch as pl\n", |
There was a problem hiding this comment.
For consistency with other examples in this PR, consider using a try-except block to support both old and new PyTorch Lightning import paths. Most other examples (e.g., in doc/source/train/doc_code/checkpoints.py and python/ray/train/examples/) use a compatibility block, while this notebook and tune-vanilla-pytorch-lightning.ipynb perform a hard switch to lightning.pytorch. Using the try-except pattern would make the examples more robust for users with different environments.
# Support both Lightning 2.x (`lightning.pytorch`) and 1.x (`pytorch_lightning`).
try:
import lightning.pytorch as pl
except ModuleNotFoundError:
import pytorch_lightning as pl
| try: | ||
| import lightning.pytorch as pl | ||
| except ModuleNotFoundError: | ||
| import pytorch_lightning as pl | ||
|
|
There was a problem hiding this comment.
To improve maintainability and avoid code duplication, consider centralizing the PyTorch Lightning import logic. This try-except block for importing pl is repeated in several test files (e.g., python/ray/train/tests/test_train_usage.py).
You could enhance ray.tune.integration.pytorch_lightning to handle this, as it already contains compatibility logic for other Lightning components.
For example, in python/ray/tune/integration/pytorch_lightning.py:
try:
import lightning.pytorch as pl
from lightning.pytorch import Callback, LightningModule, Trainer
except ModuleNotFoundError:
import pytorch_lightning as pl
from pytorch_lightning import Callback, LightningModule, TrainerThen, test files could simply import pl from this central location, making future updates easier:
from ray.tune.integration.pytorch_lightning import pl, TuneReportCheckpointCallback
doc/source/train/examples/lightning/lightning_cola_advanced.ipynb
Outdated
Show resolved
Hide resolved
Signed-off-by: Lehui Liu <lehui@anyscale.com>
doc/source/train/examples/lightning/dolly_lightning_fsdp_finetuning.ipynb
Outdated
Show resolved
Hide resolved
Signed-off-by: Lehui Liu <lehui@anyscale.com>
Signed-off-by: Lehui Liu <lehui@anyscale.com>
Signed-off-by: Lehui Liu <lehui@anyscale.com>
Description
import pytorch_lightningstill exists for backward compatibility, we have to modify our docker environment and lightning integration utilities to use the new import path.Related issues
38200
Additional information
release test: https://buildkite.com/ray-project/release/builds/81617#