Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Train] Add support for handling multiple batch data types for prepare_data_loader #26385

Closed
VishDev12 opened this issue Jul 8, 2022 · 2 comments · Fixed by #26386
Closed
Labels
bug Something that is supposed to be working; but isn't P2 Important issue, but not time-critical

Comments

@VishDev12
Copy link
Contributor

VishDev12 commented Jul 8, 2022

What happened + What you expected to happen

When working with Ray Train, using the ray.train.torch.prepare_data_loader method with a dataset that returns a dictionary instead of a tuple from its __getitem__ method causes issues.

That's because in the _WrappedDataLoader class, the _move_to_device method only deals with a tuple:

return tuple(try_move_device(i) for i in item)

def _move_to_device(self, item):
if item is None:
return None
def try_move_device(i):
try:
i = i.to(self.device, non_blocking=self._auto_transfer)
except AttributeError:
logger.debug(f"Item {i} cannot be moved to device " f"{self.device}.")
return i
with torch.cuda.stream(self._memcpy_stream):
return tuple(try_move_device(i) for i in item)

Ideally, this should deal with a dictionary type batch and an arbitrary level of nesting just like Pytorch does.

Versions / Dependencies

Python: 3.8.10
Ray: Tested with 1.13.0. Code hasn't changed in master except for a move from torch.py -> train_loop_utils.py

Reproduction script

Using any dataset that returns a dictionary from its getitem method should demonstrate this issue.

Issue Severity

Medium: It is a significant difficulty but I can work around it.

@VishDev12 VishDev12 added bug Something that is supposed to be working; but isn't triage Needs triage (eg: priority, bug/not-bug, and owning component) labels Jul 8, 2022
@xwjiang2010 xwjiang2010 added P2 Important issue, but not time-critical air and removed triage Needs triage (eg: priority, bug/not-bug, and owning component) labels Jul 8, 2022
@xwjiang2010
Copy link
Contributor

@amogkam

@truelegion47
Copy link
Contributor

truelegion47 commented Jul 9, 2022

Hi folks, I have a Draft PR for this issue. Is this what we are intending to do here for dict handling ? Please provide feedback when you get a chance.

@amogkam Thanks in advance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that is supposed to be working; but isn't P2 Important issue, but not time-critical
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants