SparK Implementation

[gabrielfruet]

Addresses #1462

Continues #1750

Inspired by: https://github.com/keyu-tian/SparK

https://arxiv.org/pdf/2301.03580

[codecov]

Codecov Report

❌ Patch coverage is 53.89408% with 148 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.78%. Comparing base (ee30cd4) to head (9687035).
⚠️ Report is 8 commits behind head on master.

Files with missing lines	Patch %	Lines
lightly/models/modules/sparse_spark.py	50.00%	145 Missing ⚠️
lightly/loss/sparse_spark.py	89.65%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1892      +/-   ##
==========================================
- Coverage   86.10%   84.78%   -1.32%     
==========================================
  Files         168      171       +3     
  Lines        6979     7375     +396     
==========================================
+ Hits         6009     6253     +244     
- Misses        970     1122     +152     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[gabrielfruet]

@liopeer If you wish i could increase test coverage. If not necessary let me know.

[gabrielfruet]

I'll take a look into these type errors (Check 3.7)

[gabrielfruet]

FYI, i still have to address these python 3.7 errors. Do we still support python 3.7?

[gabrielfruet]

Hi @liopeer , when possible, can you approve these workflows? I think i fixed most of the issues. Also, the 3.7 tests are still relevant?

Also if you have any other suggestion for the code, let me know!

[gabrielfruet]

1. Tests are green
2. Formatted the code
3. Generated notebooks

Quick question: One of my modules depends on timm. Should i port the necessary classes to our repo or should i declare it as a dependency?

from timm.models.layers import DropPath, trunc_normal_

[IgorSusmelj]

1. Tests are green
2. Formatted the code
3. Generated notebooks

Quick question: One of my modules depends on timm. Should i port the necessary classes to our repo or should i declare it as a dependency?

from timm.models.layers import DropPath, trunc_normal_

If timm is not yet a dependency we should copy over the files. You can add a note that you copied them from timm.

[gabrielfruet]

There are two options here. I was taking a look and we also have this pattern for timm's ViT to skip tests on minimal CI

lightly/lightly/utils/dependency.py

Lines 27 to 44 in 904c50a

	@functools.lru_cache(maxsize=1)
	def timm_vit_available() -> bool:
	"""Checks if Vision Transformer (ViT) models are available in the timm library.
	This function checks if the `vision_transformer` module and `LayerType` from timm
	are available, which requires timm version >= 0.3.3 and >= 0.9.9, respectively.
	Returns:
	True if the Vision Transformer (ViT) models are available in timm,
	otherwise False.
	"""
	try:
	import timm.models.vision_transformer # Requires timm >= 0.3.3
	from timm.layers import LayerType # Requires timm >= 0.9.9
	except ImportError:
	return False
	return True

We already have a test that depends on DropPath and is being just skipped if timm is not available. Since we are already using this pattern, i'll stick to it.

lightly/tests/models/test_ModelUtils.py

Lines 923 to 938 in 904c50a

	def test_update_drop_path_rate__uniform() -> None:
	pytest.importorskip("timm.models.vision_transformer")
	from timm.layers import DropPath
	from timm.models.vision_transformer import VisionTransformer
	model = VisionTransformer(drop_path_rate=0.2, depth=4)
	utils.update_drop_path_rate(model=model, drop_path_rate=0.1, mode="uniform")
	for drop_path in [
	model.blocks[0].drop_path1,
	model.blocks[0].drop_path2,
	model.blocks[-1].drop_path1,
	model.blocks[-1].drop_path2,
	]:
	assert isinstance(drop_path, DropPath)
	assert drop_path.drop_prob == 0.1

[gabrielfruet]

I tested with py37 on my environment and the workflows went green. I'm just worried about the test coverage. I'll take a look into it when possible.

[liopeer]

Don't worry too much about the test coverage. :) I can review again mid next week.

[gabrielfruet]

1. Was missing trunc_normal_ import on one of the files
2. Check 3.7 was failling due to pre 3.8 type hints.

Iterating a bit through the workflows, still getting used to it :)

[gabrielfruet]

Failed because i forgot to run make format. I opened an issue to discuss a solution for this #1908 using pre-commit.

[liopeer]

Overall I think this is mostly done. I will spend some time on Monday to rename a few variables and do smaller clean-ups – some variables are not very descriptive and I would like to change that before we make this public-facing. Thanks a lot for the great contribution!

[liopeer]

The download of Caltech101 currently does not seem to work for me. If it worked for you, ignore this comment – might open an issue with torchvision after checking if it has something to do with my local setup.

[gabrielfruet]

I know that torchvision is deprecating some datasets. So, some links are broken pytorch/vision#9335

[liopeer]

Best to include a match parameter with pytest.raises. That also serves as documentation on what the test is supposed to check.

[gabrielfruet]

I'll include!