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

Refactor Pytorch dataset #202

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jhamman
Copy link
Contributor

@jhamman jhamman commented Feb 1, 2024

Description of proposed changes

This PR introduces a number of changes to xbatcher's pytorch MapDataset class. These changes came out of the learnings while preparing my AMS talk (slides and blog post coming soon). The changes here allow for more control over how w

  • includes concurrent loading of x/y batches with dask.compute()
  • transforms now are expected to take an xarray object and return a torch.Tensor

transforms

  - includes concurrent loading of x/y batches with dask.compute()
  - transforms now are expected to return a torch.Tensor
Copy link

codecov bot commented Feb 1, 2024

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (73aabd7) 96.25% compared to head (cb2c8ab) 94.00%.

Files Patch % Lines
xbatcher/loaders/torch.py 67.85% 4 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #202      +/-   ##
==========================================
- Coverage   96.25%   94.00%   -2.25%     
==========================================
  Files           6        6              
  Lines         347      367      +20     
  Branches       82       86       +4     
==========================================
+ Hits          334      345      +11     
- Misses          8       12       +4     
- Partials        5       10       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@maxrjones
Copy link
Member

maxrjones commented Feb 5, 2024

Thanks for contributing these improvements @jhamman! It would be good to hit the test coverage target, so do you mind if I push additional tests as part of the review?

@maxrjones
Copy link
Member

Sorry that I did not have time to review this yet! I am about to head out of office but would be glad to work on this with you after returning on March 5, or would welcome other reviewers in the meantime.

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