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

Cordiff usability and performance enhancements for custom dataset training #790

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

Conversation

CharlelieLrt
Copy link
Collaborator

@CharlelieLrt CharlelieLrt commented Feb 11, 2025

Modulus Pull Request

Description

This PR provides enhancements for usability of CorrDiff training on custom datasets. It includes:

  • Documentation improvements (readme, docstrings)
  • A new general purpose patching API with enhanced usability and performance
  • Refactor CorrDiff samplers and losses to delegate all patching logic to the dedicated API
  • Refactor CorrDiff UNets positional embeddings for performance
  • Config files refactoring and simplification
  • Legacy code elimination
  • Support for non-square images
  • Experimental support for non-square patches
  • Other bugfixes

Closes #726.
Closes #447.
Closes #417.
Closes #448.

Note

This PR introduces breaking changes on the way corrdiff config files are handled.
For training, older version of the config files is still supported.
For generation, a new parameter generation.patching: true/false is required.

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

@CharlelieLrt CharlelieLrt added bug Something isn't working Documentation Improvements or additions to documentation enhancement New feature or request 2 - In Progress Currently a work in progress labels Feb 11, 2025
@CharlelieLrt CharlelieLrt self-assigned this Feb 11, 2025
@CharlelieLrt
Copy link
Collaborator Author

/blossom-ci

@CharlelieLrt
Copy link
Collaborator Author

/blossom-ci

@CharlelieLrt
Copy link
Collaborator Author

/blossom-ci

@CharlelieLrt
Copy link
Collaborator Author

/blossom-ci

@CharlelieLrt CharlelieLrt changed the title Cordiff usability enhancements for custom dataset training Cordiff usability and performance enhancements for custom dataset training Mar 4, 2025
@CharlelieLrt
Copy link
Collaborator Author

/blossom-ci

@pzharrington
Copy link
Collaborator

Overall this is looking nice, thanks for doing a lot of cleanup and docstring/readme improvements! I checked for compatibility/overlap with other diffusion weather models (StormCast, ReGen) and it looks fine in that regard. I had a couple minor comments about the unit tests for patching utilities but other than that no major qualms. It would be good to address Akshay's comment about performance if we can, it does seem like avoiding allocating the full meshgrid is worth doing since the whole goal is to avoid too much memory allocation and ops at the full image resolution

@CharlelieLrt
Copy link
Collaborator Author

/blossom-ci

@CharlelieLrt
Copy link
Collaborator Author

@pzharrington thanks for your comments! I made the patching tests more robust and addressed @akshaysubr suggestions for performance improvements.

@CharlelieLrt
Copy link
Collaborator Author

/blossom-ci

@pzharrington pzharrington self-requested a review March 7, 2025 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team bug Something isn't working Documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
3 participants