Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Add support for Lightning 1.8 + Fixes for the CI #1479

Merged
merged 39 commits into from
Nov 5, 2022
Merged

Add support for Lightning 1.8 + Fixes for the CI #1479

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
7d067c8
Add requires(icevision) for loading data (icevision integration)
krshrimali Oct 26, 2022
cffc3ab
Merge branch 'master' of github.com:Lightning-AI/lightning-flash
krshrimali Nov 3, 2022
da73676
checking if issues were there for pl 1.7.7
krshrimali Nov 3, 2022
4438093
Update requirements.txt
krshrimali Nov 3, 2022
f189866
Pin sahi version
krshrimali Nov 3, 2022
2e98376
list dependencies as a separate step
krshrimali Nov 4, 2022
0bad50f
Sahi pin to 0.8.19 for serve
krshrimali Nov 4, 2022
abc6ef2
try torchscript instead of torch.jit.trace
krshrimali Nov 4, 2022
b180bed
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 4, 2022
23c4f1e
Update golang
krshrimali Nov 4, 2022
8757e7c
Merge branch 'debug-lightning-18' of github.com:Lightning-AI/lightnin…
krshrimali Nov 4, 2022
1178289
1.17
krshrimali Nov 4, 2022
e125707
1.17.9
krshrimali Nov 4, 2022
105c829
rollback some unrequired changes
krshrimali Nov 4, 2022
21bd12a
@rohitgr7 suggested this
krshrimali Nov 4, 2022
bfec178
@rohitgr7 suggested this
krshrimali Nov 4, 2022
fd304eb
lr_schedulers to lr_scheduler_configs
krshrimali Nov 4, 2022
82d7785
check if PL greater than equal to 1.8.0
krshrimali Nov 4, 2022
7533ec8
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 4, 2022
563bbfb
add PL check for 1.8.0
krshrimali Nov 4, 2022
81e54f6
Merge branch 'debug-lightning-18' of github.com:Lightning-AI/lightnin…
krshrimali Nov 4, 2022
3d2e791
Start 3.8 to 3.10
krshrimali Nov 4, 2022
3733b5b
Move back to 3.7 to 3.9
krshrimali Nov 4, 2022
30deeb6
Remove _notebooks as it's unused
krshrimali Nov 4, 2022
7ead5d1
Revert "Remove _notebooks as it's unused"
krshrimali Nov 4, 2022
e62ed60
fix lr_scheduler_configs usage
krshrimali Nov 4, 2022
5ecaa31
try fixing jsonnet (go < 1.17)
krshrimali Nov 4, 2022
f9769e1
Merge branch 'debug-lightning-18' of github.com:Lightning-AI/lightnin…
krshrimali Nov 4, 2022
e085737
Try installing 1.17
krshrimali Nov 4, 2022
da7a080
Try installing 1.17
krshrimali Nov 4, 2022
5e5af42
Try installing 1.17
krshrimali Nov 4, 2022
4583ef6
Try installing 1.17
krshrimali Nov 4, 2022
faa3b17
Try installing 1.17
krshrimali Nov 4, 2022
a810e7a
Skip mac os 12 (flaky)
krshrimali Nov 4, 2022
88b1e67
skip flaky test on Mac
krshrimali Nov 4, 2022
7e39c50
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 4, 2022
77d369d
Merge branch 'master' into debug-lightning-18
krshrimali Nov 4, 2022
49cfb60
Update with num_devices instead of tpu_cores
krshrimali Nov 4, 2022
c506d59
Add CHANGELOG
krshrimali Nov 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions .github/workflows/ci-testing.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ jobs:
pip install torch>=1.7.1
pip install '.[${{ join(matrix.topic, ',') }}]' --upgrade $flag --find-links https://download.pytorch.org/whl/cpu/torch_stable.html
pip install '.[test]' --upgrade
pip list
shell: bash

- name: Install vissl
Expand All @@ -128,7 +127,7 @@ jobs:
if: contains( matrix.topic , 'serve' )
run: |
sudo apt-get install libsndfile1
pip install '.[all,audio]' icevision effdet --upgrade
pip install '.[all,audio]' icevision sahi==0.8.19 effdet --upgrade
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why icevision, sahi, effdet are not part of the requirements?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are. We don't need these modules for serve though, but for serve testing we need them. Hence explicit installation. @Borda

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pip install '.[all,audio]' icevision sahi==0.8.19 effdet --upgrade
pip install '.[all,audio,serve]' --upgrade

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but still why all does not include all?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uhmm, because we only need icevision, effdet for the serve tests. And these are in requirements/datatype_image_extras.txt (which has a lot more than we need for serve tests).

So basically:

  • icevision, effdet are not needed for serve in Lightning Flash.
  • icevision, effdet are only needed for "serve tests" in Flash.

Hence the explicit mention of sahi, icevision, effdet (sahi because we need to pin it).

Does that help, @Borda ? Please let me know if you have any questions or suggestions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still, sahi, icevision, effdet shall be in a requirement file for reproducibility in case you would need to restrict versions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are here: https://github.com/Lightning-AI/lightning-flash/blob/c506d590aebb9e339b159c6aa0d48c97ec1fb733/requirements/datatype_image_extras.txt#L4-L8

And we already use this^ for image.

And this is only temporary, once icevision is fixed for the latest sahi release, this will go.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's do these requirements cleaning in another PR, and lets link this discussion 🦦

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! ^ From an off-line discussion with @Borda, I fully agree that we should make reproducible environments for users to have the same testing environment as the CI. Nobody will read the CI dependencies to figure out why it fails in the CI.

So, we'll do something similar to what TorchMetrics has: https://github.com/Lightning-AI/metrics/tree/master/requirements

I'll make a PR and link the discussion here. Thank you, @Borda!!


- name: Install audio test dependencies
if: contains( matrix.topic , 'audio' )
Expand All @@ -137,6 +136,11 @@ jobs:
pip install matplotlib
pip install '.[audio,image]' torch==1.11.0 --upgrade

- name: PIP List
run: |
pip list
shell: bash

- name: Cache datasets
uses: actions/cache@v3
with:
Expand Down
3 changes: 2 additions & 1 deletion flash/core/integrations/icevision/data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from flash.core.data.utilities.loading import IMG_EXTENSIONS, load_image, NP_EXTENSIONS
from flash.core.data.utilities.paths import list_valid_files
from flash.core.integrations.icevision.transforms import from_icevision_record
from flash.core.utilities.imports import _ICEVISION_AVAILABLE
from flash.core.utilities.imports import _ICEVISION_AVAILABLE, requires

if _ICEVISION_AVAILABLE:
from icevision.core.record import BaseRecord
Expand All @@ -33,6 +33,7 @@ class IceVisionInput(Input):
num_classes: int
labels: list

@requires("icevision")
def load_data(
self,
root: str,
Expand Down
1 change: 1 addition & 0 deletions requirements/datatype_image_extras.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ fiftyone
classy_vision
vissl>=0.1.5
icevision>=0.8
sahi >=0.8.19,<0.11.0
icedata
effdet
kornia>=0.5.1
Expand Down