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

Add scaling for HPX dataloaders, code cleanup #721

Merged
merged 13 commits into from
Dec 5, 2024

Conversation

daviddpruitt
Copy link
Collaborator

@daviddpruitt daviddpruitt commented Nov 20, 2024

Modulus Pull Request

Description

This adds scaling values for constant values in Healpix datasets.
If scaling values are given they will also be required for constant fields.
Removed duplicate code from coupled healpix dataloader to eliminate differing behaviour.
Closes #716

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.
  • [] The CHANGELOG.md is up to date with these changes.
  • An issue is linked to this pull request.

Dependencies

@daviddpruitt daviddpruitt marked this pull request as ready for review November 22, 2024 19:06
@daviddpruitt
Copy link
Collaborator Author

/blossom-ci

@daviddpruitt
Copy link
Collaborator Author

/blossom-ci

@pzharrington pzharrington self-requested a review December 3, 2024 17:33
@akshaysubr akshaysubr self-requested a review December 3, 2024 17:44
Copy link
Collaborator

@akshaysubr akshaysubr left a comment

Choose a reason for hiding this comment

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

Looks good to me overall. Just had a few minor comments.

modulus/datapipes/healpix/data_modules.py Show resolved Hide resolved
modulus/datapipes/healpix/timeseries_dataset.py Outdated Show resolved Hide resolved
modulus/datapipes/healpix/timeseries_dataset.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@pzharrington pzharrington left a comment

Choose a reason for hiding this comment

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

Overall looks good, but echoing Akshay's comments on modulus/datapipes/healpix/timeseries_dataset.py

@daviddpruitt
Copy link
Collaborator Author

/blossom-ci

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