Skip to content

Conversation

@drivanov
Copy link

A crash was observed due to the usage of the deprecated device parameter.

../opt/pyg/pytorch_geometric/test/test_edge_index.py::test_data_loader[True-0-int64] DeprecationWarning: The argument 'device' of Tensor.pin_memory() is deprecated. Please do not pass this argument. (Triggered internally at /opt/pytorch/pytorch/aten/src/ATen/native/Memory.cpp:46.)
DeprecationWarning: The argument 'device' of Tensor.is_pinned() is deprecated. Please do not pass this argument. (Triggered internally at /opt/pytorch/pytorch/aten/src/ATen/native/Memory.cpp:31.)
Traceback (most recent call last):
  File "/usr/local/lib/python3.12/dist-packages/torch/utils/data/_utils/pin_memory.py", line 91, in pin_memory
    clone[i] = pin_memory(item, device)
               ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/dist-packages/torch/utils/data/_utils/pin_memory.py", line 57, in pin_memory
    return data.pin_memory(device)
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/dist-packages/torch_geometric/edge_index.py", line 1211, in __torch_dispatch__
    return HANDLED_FUNCTIONS[func](*args, **(kwargs or {}))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: _pin_memory() takes 1 positional argument but 2 were given

This PR removes its use to ensure compatibility with current PyTorch versions

@pytorch-bot
Copy link

pytorch-bot bot commented Sep 16, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/163102

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit be83b24 with merge base 86db4de (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@pytorch-bot pytorch-bot bot added the release notes: dataloader release notes category label Sep 16, 2025
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 16, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@zou3519 zou3519 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Sep 18, 2025
@eqy
Copy link
Collaborator

eqy commented Sep 19, 2025

Would it be possible to add a test for this?

@drivanov
Copy link
Author

Would it be possible to add a test for this?

6 tests added

@eqy eqy added the ciflow/trunk Trigger trunk jobs on your pull request label Sep 23, 2025
@eqy
Copy link
Collaborator

eqy commented Sep 23, 2025

Does it make sense to just add the tests to test_dataloader.py rather than creating a new file? https://github.com/pytorch/pytorch/blob/ebddbe787a1d193a56b3ed60986726b7a4b61ba3/test/test_dataloader.py

@drivanov
Copy link
Author

Does it make sense to just add the tests to test_dataloader.py rather than creating a new file? https://github.com/pytorch/pytorch/blob/ebddbe787a1d193a56b3ed60986726b7a4b61ba3/test/test_dataloader.py

I can make that change if you prefer, although I noticed that test_dataloader.py is already quite large (≈approximately 3680 lines).

@pytorch-bot pytorch-bot bot removed the ciflow/trunk Trigger trunk jobs on your pull request label Sep 23, 2025
@drivanov
Copy link
Author

@eqy : Could you, please, review this PR?

@eqy eqy requested a review from albanD October 29, 2025 20:20
Copy link
Contributor

@divyanshk divyanshk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@drivanov Seconding @eqy here. lets add it to test_dataloader.py it already has other unit tests on pin memory. We can regroup tests here later.

Also, might have to check if we should update the docstring as well

@drivanov
Copy link
Author

@eqy : I moved the 6 pin_memory tests to test_dataloader.py

Copy link
Collaborator

@ngimel ngimel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the future, AI assistance on PRs has to be disclosed

github-merge-queue bot pushed a commit to NVIDIA/bionemo-framework that referenced this pull request Nov 12, 2025
Bumps the recipes container to 25.10.

We have to xfail / disable a couple of features to get the
nvidia-internal torch fork to work:
* megatron-fsdp's `gather_uneven_dtensor_to_full_tensor` seems to break
with newer versions of torch, fix WIP.
* torchdata's StatefulDataLoader uses internal pytorch APIs that changed
with pytorch/pytorch#163102 (merged in the nvidia NGC images), so to use
StatefulDataLoader we need to set `pin_memory=False`, which impacts
performance. We now have the option to fall back to the standard pytorch
dataloader with `use_stateful_dataloader=False`

---------

Signed-off-by: Peter St. John <pstjohn@nvidia.com>
savitha-eng added a commit to NVIDIA/bionemo-framework that referenced this pull request Nov 15, 2025
- Added use_stateful_dataloader: false to all hydra configs (matches ESM2)
- Updated train_ddp.py and train_fsdp2.py to conditionally pass dataloader to checkpoint functions
- Updated test_distributed_checkpointing.py to enable stateful dataloader in all tests
- Works around pin_memory issue (pytorch/pytorch#163102) by defaulting to regular DataLoader
- Tests can still validate full checkpoint/resume with use_stateful_dataloader=true

Signed-off-by: savitha-eng <savithas@nvidia.com>
savitha-eng added a commit to NVIDIA/bionemo-framework that referenced this pull request Nov 15, 2025
- Added use_stateful_dataloader: false to all hydra configs (matches ESM2)
- Updated train_ddp.py and train_fsdp2.py to conditionally pass dataloader to checkpoint functions
- Updated test_distributed_checkpointing.py to enable stateful dataloader in all tests
- Works around pin_memory issue (pytorch/pytorch#163102) by defaulting to regular DataLoader
- Tests can still validate full checkpoint/resume with use_stateful_dataloader=true

Signed-off-by: savitha-eng <savithas@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open source release notes: dataloader release notes category triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants