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

🆕 Integrate Foundation Models Available VIA timm: UNI, Virchow, Hibou, H-optimus-0 #856

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

GeorgeBatch
Copy link
Contributor

Making a pull request as discussed in issue #855


Copied from the issue:

I think it would be useful to integrate pre-trained foundation models from other labs into tiatoolbox.models.architecture.vanilla.py.

Currently, the _get_architecture() function allows the use of models from torchvision.models.

But another function _get_timm_architecture() could be made to incorporate foundation models which are available from timm with weights on HuggingFace Hub. All the models from timm that I've used require users to sign the licence agreement with the authors, so the licencing question seems to be solved itself since there is no way users will get access to the model weights just through Tiatoolbox without getting the access request approved by the authors first.

@GeorgeBatch
Copy link
Contributor Author

Only added UNI and Prov-GigaPath for now. Will add more after initial comments.

I do not like that TimmBackbone pretty much repeats the CNNBackbone. The only difference is the absence of global average pooling in TimmBackbone.

@GeorgeBatch
Copy link
Contributor Author

I found this file for testing: https://github.com/TissueImageAnalytics/tiatoolbox/blob/develop/tests/models/test_arch_vanilla.py

There might be a problem regarding memory and compute resources when running some of the larger feature extractors through GitHub actions, e.g. Prov-GigaPath needs a considerable amount of memory just to be loaded.

@shaneahmed
Copy link
Member

I found this file for testing: https://github.com/TissueImageAnalytics/tiatoolbox/blob/develop/tests/models/test_arch_vanilla.py

There might be a problem regarding memory and compute resources when running some of the larger feature extractors through GitHub actions, e.g. Prov-GigaPath needs a considerable amount of memory just to be loaded.

As this is just testing the functionality and not loading weights, I hope this would work.

@shaneahmed
Copy link
Member

shaneahmed commented Sep 3, 2024

Only added UNI and Prov-GigaPath for now. Will add more after initial comments.

I do not like that TimmBackbone pretty much repeats the CNNBackbone. The only difference is the absence of global average pooling in TimmBackbone.

In that case, you can inherit CNNBackbone and just define the functions which change like we are doing here

class PatchPredictor(EngineABC):
for PatchPredictor. The PatchPredictor uses all the functionalities of EngineABC other than the ones defined explicitly.

@shaneahmed shaneahmed changed the title Integrate foundation models available through timm: UNI, Virchow, Hibou, H-optimus-0, etc. 🆕 Integrate Foundation Models Available VIA timm: UNI, Virchow, Hibou, H-optimus-0 Sep 3, 2024
@shaneahmed shaneahmed added this to the Release v1.6.0 milestone Sep 18, 2024
@shaneahmed shaneahmed added the enhancement New feature or request label Sep 18, 2024
@shaneahmed
Copy link
Member

I have updated this branch to make sure that tests pass on Ubuntu-24 before we merge it with develop.

GeorgeBatch and others added 4 commits September 24, 2024 16:41
- remove explicit assert statement for `timm` version
- add `timm` version into in if statement for prov-gigapath
- add comment about `timm` version for timm-based models

Co-authored-by: Shan E Ahmed Raza <13048456+shaneahmed@users.noreply.github.com>
was not removed while accepting suggested changes
"hf_hub:prov-gigapath/prov-gigapath", pretrained=True
)
else:
msg = f"Architecture {arch_name} not supported. If you are loading timm models, only timm version > `1.0.3` are supported."
Copy link
Member

Choose a reason for hiding this comment

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

This line is too long and causing the tests to fail. If you do pre-commit install in your development environment then it will test formatting before making a commit. You can also run ruff manually in your terminal before making a commit.

Suggested change
msg = f"Architecture {arch_name} not supported. If you are loading timm models, only timm version > `1.0.3` are supported."
msg = f"Architecture {arch_name} not supported."
f"If you are loading timm models, only timm version > `1.0.3` are supported."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrate foundation models available through timm: UNI, Virchow, Hibou, H-optimus-0, etc.
2 participants