ViCCo-Group / thingsvision

Python package for extracting representations from state-of-the-art computer vision models
https://vicco-group.github.io/thingsvision/
MIT License
157 stars 21 forks source link

74 implement low memory option for feature extraction #96

Closed andropar closed 2 years ago

andropar commented 2 years ago
LukasMut commented 2 years ago

@andropar Please add documentation about this new feature in the README.md and our official docs. Looks good so far :) The default option should stay as what we have had before (i.e., extracting all features without partitioning them into subsets)

andropar commented 2 years ago

Will do after Philipp revamped the docs 👍

codecov-commenter commented 2 years ago

Codecov Report

Base: 80.75% // Head: 79.94% // Decreases project coverage by -0.81% :warning:

Coverage data is based on head (6bec534) compared to base (b4363f7). Patch coverage: 63.41% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #96 +/- ## ========================================== - Coverage 80.75% 79.94% -0.82% ========================================== Files 23 23 Lines 1029 1042 +13 Branches 148 150 +2 ========================================== + Hits 831 833 +2 - Misses 164 172 +8 - Partials 34 37 +3 ``` | Flag | Coverage Δ | | |---|---|---| | unittests | `79.94% <63.41%> (-0.82%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ViCCo-Group#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/ViCCo-Group/THINGSvision/pull/96?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ViCCo-Group) | Coverage Δ | | |---|---|---| | [thingsvision/core/extraction/base.py](https://codecov.io/gh/ViCCo-Group/THINGSvision/pull/96/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ViCCo-Group#diff-dGhpbmdzdmlzaW9uL2NvcmUvZXh0cmFjdGlvbi9iYXNlLnB5) | `70.37% <50.00%> (-15.35%)` | :arrow_down: | | [thingsvision/core/extraction/mixin.py](https://codecov.io/gh/ViCCo-Group/THINGSvision/pull/96/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ViCCo-Group#diff-dGhpbmdzdmlzaW9uL2NvcmUvZXh0cmFjdGlvbi9taXhpbi5weQ==) | `80.19% <78.94%> (-1.12%)` | :arrow_down: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ViCCo-Group). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ViCCo-Group)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

andropar commented 2 years ago

No specific reason, what do you think the default should be?

LukasMut commented 2 years ago

No specific reason, what do you think the default should be?

I am not sure. I was thinking of a scenario where there are not even 100 batches. Let's assume the batch size is set to 32 or 64. A default of 100 would mean that there are at least 3200 or 6400 images in the dataset. This is not a tiny dataset. However, for smaller datasets, it's probably ok to avoid the low memory option entirely and simply resort to the default. What do you think?

andropar commented 2 years ago

I thought about this too, its a tradeoff between batch size, number of batches, number of features and feature size. Maybe we can use some kind of heuristic as a default, e.g. assuming 8GB available RAM and then approximating how much would fit into memory?

LukasMut commented 2 years ago

I thought about this too, its a tradeoff between batch size, number of batches, number of features and feature size. Maybe we can use some kind of heuristic as a default, e.g. assuming 8GB available RAM and then approximating how much would fit into memory?

I like that idea. This sounds reasonable to me. We can probably even assume 16GB of available RAM. Every local machine these days has approx. 16 - 32 GB of RAM. The only problem with that approach might be the variability of the size of the activations across layers (early layers consume substantially more space than later layers). However, it's a trade-off as you've just said. We need to make a choice. I think this is a fine choice.