ContinualAI / ContinualAI/avalanche

make_train_dataloader discards custom collate function passed as kwarg

Open
#1,531 5 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Python
Stars
2.1k
Forks
321
PR merge metrics
No merged PRs in 30d

Description

**Describe the bug**
Calling
```python
cl_strategy.train(
experience,
eval_streams=[val_exp],
num_workers=4,
collate_fn=my_custom_collate,
)
```

should respect all of the keyword arguments I pass in. **In this case, `my_custom_collate` is discarded.**

**To Reproduce**
For debugging, I define a custom strategy to examine what is passed into the dataloader. The make_train_dataloader function is lifted as it is from the [0.4.0 implementation](https://github.com/ContinualAI/avalanche/blob/fe1c098fb4eacc394499f0e3dce51a6fbc66cfdc/avalanche/training/templates/base_sgd.py#L376C23-L376C23)

_(Please note that I set breakpoints with `pdb`)_

```python
class CustomNaiveStrategy(Naive):
def make_train_dataloader(
self,
num_workers=0,
shuffle=True,
pin_memory=None,
persistent_workers=False,
drop_last=False,
**kwargs
):
assert self.adapted_dataset is not None

# fmt:off
import pdb; pdb.set_trace();
# fmt:on

other_dataloader_args = self._obtain_common_dataloader_parameters(
batch_size=self.train_mb_size,
num_workers=num_workers,
shuffle=shuffle,
pin_memory=pin_memory,
persistent_workers=persistent_workers,
drop_last=drop_last,
)

# fmt:off
pdb.set_trace()
# fmt:on

if "ffcv_args" in kwargs:
other_dataloader_args["ffcv_args"] = kwargs["ffcv_args"]

self.dataloader = TaskBalancedDataLoader(
self.adapted_dataset, oversample_small_groups=True, **other_dataloader_args
)
```

**Expected behavior**

`other_dataloader_args` should obey the kwargs and pass `my_custom_collate` along

**Screenshots**
![bug](https://github.com/ContinualAI/avalanche/assets/41016622/c1ae7855-751b-4a66-9efb-7a9eebc3c2ae)

In the screenshot above `p kwargs` shows the custom collate function, but that does not show up in `other_dataloader_args` which is what is passed onto `TaskBalancedDataLoader`

**Additional context**

I cannot immediately think of why something like
`other_dataloader_args.update(kwargs)` is a poor idea, would love to hear thoughts.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.