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

Fix plotting in datamodules when dataset is a Subset #2003

Merged
merged 2 commits into from
Apr 17, 2024
Merged

Conversation

calebrob6
Copy link
Member

We have a .plot(...) method in our BaseDataModule that routes plot requests from the trainers through to the dataset object's .plot(...) method. This currently only works if trainer.dataset or trainer.val_dataset has a plot method. However, when you call torch.utils.data.random_split to get a train/val split, your dataset objects are Subsets which don't have a plot method.

@github-actions github-actions bot added the datamodules PyTorch Lightning datamodules label Apr 16, 2024
@calebrob6
Copy link
Member Author

I can confirm that with this change my plots show up in tensorboard 👍

@adamjstewart
Copy link
Collaborator

Very clever, don't know why I never thought of this.

Do any of our builtin datamodules suffer from this issue? Just want to make sure we have 100% test coverage.

@adamjstewart adamjstewart added this to the 0.5.3 milestone Apr 17, 2024
@robmarkcole
Copy link
Contributor

I've def experienced this issue, likely OSCD or levir

@adamjstewart
Copy link
Collaborator

Ah, I just noticed codecov actually works for this PR since it doesn't come from a fork.

@adamjstewart adamjstewart merged commit 2f46c73 into main Apr 17, 2024
17 checks passed
@adamjstewart adamjstewart deleted the plotting_fix branch April 17, 2024 09:21
@adamjstewart adamjstewart modified the milestones: 0.5.3, 0.6.0 Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
datamodules PyTorch Lightning datamodules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants