Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The new lightly release breaks BaseTask with timm imports #1824

Closed
CarlosGomes98 opened this issue Jan 24, 2024 · 5 comments · Fixed by #1825
Closed

The new lightly release breaks BaseTask with timm imports #1824

CarlosGomes98 opened this issue Jan 24, 2024 · 5 comments · Fixed by #1825
Milestone

Comments

@CarlosGomes98
Copy link

Description

The new 1.4.26 release of lightly (https://pypi.org/project/lightly/#history), pulled by default when installing torchgeo, tries to run

from timm.layers import LayerType, Mlp, PatchEmbed

with results in the error:

ImportError: cannot import name 'LayerType' from 'timm.layers' (/dccstor/geofm-finetuning/carlosgomes/torchgeo/lib/python3.10/site-packages/timm/layers/__init__.py) 

This can be fixed by locking the version for lightly

Steps to reproduce

Import any trainer that inherits from BaseTask

Version

0.5.1

@adamjstewart
Copy link
Collaborator

Thanks for the bug report! We also noticed this when dependabot tried to update our lightly dependency in #1823. Here is a rundown of how this bug surfaced. It's actually pretty complicated (and interesting!)

The issue started when lightly v1.4.26 added an optional dependency on timm in lightly-ai/lightly#1479. If timm is available, lightly exposes additional parts of the API and imports certain features of timm. However, the features it uses are only available starting in timm v0.9.9: huggingface/pytorch-image-models#1996.

At the same time, TorchGeo depends on segmentation-models-pytorch, which requires timm v0.9.2: https://github.com/qubvel/segmentation_models.pytorch/blob/v0.3.3/requirements.txt#L4. We've asked them many times not to pin to a specific version of timm, but to no avail. The library itself may not even be maintained anymore, there haven't been any updates in quite some time.

So the short-term fix is to pin TorchGeo to depend on lightly<=1.4.25. This will be included in the next TorchGeo patch release, likely some time in the next week. A medium-term fix would be for lightly to first check the version of timm available and only try to use it if it's timm v0.9.9+. I'm not sure if they will be interested in the additional code complexity to check this, but I'll suggest it anyway. A long-term fix would be to somehow resurrect or replace segmentation-models-pytorch so we can finally use modern versions of timm. I'll also explore this, but I won't get my hopes up.

@adamjstewart
Copy link
Collaborator

adamjstewart commented Jan 25, 2024

For anyone who encounters this issue, the current workaround is:

pip install torchgeo lightly!=1.4.26

It looks like this is going to be fixed in the next lightly release: lightly-ai/lightly#1487

@adamjstewart
Copy link
Collaborator

This is now fixed in main and will be included in the next release. It's also been fixed in lightly itself (lightly-ai/lightly#1487), so future lightly releases will be fine. We're still considering migrating from smp to torchseg to remove the timm version constraint, which will also alleviate this issue.

@CarlosGomes98
Copy link
Author

CarlosGomes98 commented Feb 2, 2024 via email

@adamjstewart
Copy link
Collaborator

FYI, lightly 1.5.0 has now been released and fixes this bug.

@adamjstewart adamjstewart added this to the 0.5.2 milestone Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants