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

Set upper bound of PyTorch version #1178

Merged
merged 2 commits into from
Jun 14, 2024
Merged

Set upper bound of PyTorch version #1178

merged 2 commits into from
Jun 14, 2024

Conversation

fepegar
Copy link
Owner

@fepegar fepegar commented Jun 14, 2024

Description

This doesn't work

import torch
import torchio as tio

subject = tio.datasets.Colin27()
subject.load()
subjects = 10 * [subject]

from torch.utils.data import DataLoader
loader = DataLoader(subjects, batch_size=4)
batch = next(iter(loader))
batch.__class__ is dict

because batch is an instance of Subject.

This happens in PyTorch >=2.3 because of

This PR forces PyTorch to be <2.3, temporarily.

Checklist

  • I have read the CONTRIBUTING docs and have a developer setup (especially important are pre-commitand pytest)
  • Non-breaking change (would not break existing functionality)
  • Breaking change (would cause existing functionality to change)
  • Tests added or modified to cover the changes
  • Integration tests passed locally by running pytest
  • In-line docstrings updated
  • Documentation updated, tested running make html inside the docs/ folder
  • This pull request is ready to be reviewed

@fepegar fepegar changed the title Add failing test Set upper bound of PyTorch version Jun 14, 2024
@fepegar fepegar marked this pull request as ready for review June 14, 2024 19:15
@fepegar fepegar merged commit 46b2906 into main Jun 14, 2024
18 checks passed
@fepegar fepegar deleted the fix-subject-copy-issue branch June 14, 2024 19:18
@fepegar
Copy link
Owner Author

fepegar commented Jun 14, 2024

@justusschock, do you think you could take a look at this problem? As you worked on the logic for copying subjects. If not, that's absolutely fine, but please let me know.

@fzimmermann89
Copy link

@fepegar : Maybe a stupid question, but why is this important?

isinstance(batch,dict) is still true, as Subject is a subclass of dict.
This seems quite logical and works as I would expect.

(I am asking because we would like to keep using torchio with pytorch 2.3 and would be willing to fix the issue -- if I could understand why this should be an issue and what the expected behavior is)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants