Pin memory in Trainer by default

[abhishekkrthakur]

No description provided.

[sgugger]

Thanks for adding this!

[stas00]

Could we please go through normal PR review approval cycles? Unless I missed something and there was one.

It looks like my comment on slack was missed where I suggested to use a more specific cl arg name.

I proposed one of:

• dataloader_pin_memory
• dl_pin_memory

But since we already have dataloader_num_workers:

            num_workers=self.args.dataloader_num_workers,
            pin_memory=self.args.pin_memory,

it should probably be dataloader_pin_memory

This is important since there are other ways to pin memory in pytorch.

---

This is a general comment - not specific to this PR:

We have this ongoing issue wrt cl arg naming, that we name something and later we realize it's not the best name and then we are concerned with changing the name not to break user's code, so let's think deeply about new cl args names before we add them. Thank you!

[abhishekkrthakur]

@stas00 It seems like I missed this message and when I opened this PR in the morning, I didn't see any comments and @sgugger had approved the PR. For a final check, I asked @LysandreJik who gave me the green light.

To avoid this in future, I would request if PR specific comments are made on the PR itself so that author & other reviewers can go through them and make sure that everything is resolved before merging.

[stas00]

Yes, absolutely. I guess it just fell through the cracks.

And let's have PR description, as simple as:

This PR adds --pin_memory to trainer DataLoader and it defaults to True.