Enable Offline-ER for NestedTensors #336
Conversation
Coverage reportThe coverage rate went from
Diff Coverage details (click to unfold)src/renate/updaters/learner_components/losses.py
src/renate/utils/pytorch.py
src/renate/updaters/experimental/offline_er.py
|
| """Given a NestedTensor, return its batch size.""" | ||
| if isinstance(batch, torch.Tensor): | ||
| return batch.shape | ||
| if isinstance(batch, tuple): |
There was a problem hiding this comment.
are we assuming all the tensors in the tuple have the same shape? or that only the first one actually contains data? I think it's important to make this clear.
There was a problem hiding this comment.
You are right, technically they could be of different shapes. I've renamed the function to reflect that it now returns the first dim only and the docstring to say that we expect the first dim to match.
| assert get_shape_nested_tensors(dict_tensor)[0] == expected_batch_size | ||
|
|
||
|
|
||
| def test_cat_nested_tensors(): |
There was a problem hiding this comment.
It would be good to test also the behavior in case of failure (e.g., shape mismatch)
Offline-ER was expecting torch.Tensors to be returned by the dataloader which is oftentimes not the case, e.g., text data with transformers.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.