Closed MalteEbner closed 1 month ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 82.19%. Comparing base (
c890e10
) to head (bbc2958
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Tests for losses
test_dist__gather.py
totest_dist__gather__losses.py
.NTXentLoss
, but alsoTiCoLoss
,VICRegLoss
andDCLLoss
VICRegLLoss
, as it requires the global and local features and grids as input, which does not fit into the current framework. I think the effort to also test it is not worth it for the moment. If we want to do it, we should do it in a follow-up issue.gather(...)
Aligning the import style is another topic. Here it is a bit more difficult, as there is the name clash between
lightly.utils.dist
andtorch.dist
. Currently some files import the one and some the other asdist
.Test for Benchmark module
Added
test_dist__gather__benchmark_module.py
for benchmarking theBenchmarkModule
. Also fixed bug in theBenchmarkModule.on_validation_epoch_start
: As theself.dataloader_kNN
is not split by the DDP, all devices have the full dataloader and no gathering is necessary.Other changes
Some typing updates
Missing
No missing test for
lightly.utils.dist.gather
All usages of lightly.utils.dist.gather are covered now (except
![image](https://github.com/lightly-ai/lightly/assets/20324507/179dcbd6-480e-41de-957d-7f78d20ac7a3)
VICRegLLoss
):Missing test for other DDP functions
We also have more gather-like functions, e.g.![image](https://github.com/lightly-ai/lightly/assets/20324507/7efbb9fd-1a00-42eb-b20d-659149d59778)
concat_all_gather
inlightly/models/utils.py
.And the kNN also use some gather functions:![image](https://github.com/lightly-ai/lightly/assets/20324507/81b9faee-5458-48ff-9f85-c27c0483ad65)
However, these are out of scope for this issue.