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.

Sign up for GitHub

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

Jump to bottom

CorrDiff Refactoring Part II #629

Merged
merged 20 commits into from
Aug 13, 2024
Merged

CorrDiff Refactoring Part II #629

merged 20 commits into from
Aug 13, 2024

Conversation

mnabian
Copy link
Collaborator

@mnabian mnabian commented Aug 5, 2024 •
edited
Loading

Modulus Pull Request

Description

  • Refactor the generation script.
  • Update the README
  • Remove class conditioning
  • Better handling of the configs for generation

Closes #313, #633, #639

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

@mnabian mnabian self-assigned this Aug 5, 2024
@mnabian mnabian changed the title CorrDiff: Refactor inference CorrDiff Refactoring Part II Aug 7, 2024
nbren12
nbren12 reviewed Aug 9, 2024
View reviewed changes
Copy link
Collaborator

@nbren12 nbren12 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 to me. It's good to flatten the function calls in generate.py into a single main.

modulus/utils/corrdiff/utils.py Show resolved Hide resolved
jleinonen
jleinonen approved these changes Aug 12, 2024
View reviewed changes
Copy link
Collaborator

@jleinonen jleinonen left a comment

Choose a reason for hiding this comment

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

I left a few comments, but overall looks very good!

examples/generative/corrdiff/README.md Show resolved Hide resolved
examples/generative/corrdiff/conf/config_generate.yaml Outdated Show resolved Hide resolved
examples/generative/corrdiff/train.py Outdated Show resolved Hide resolved
modulus/models/diffusion/preconditioning.py Show resolved Hide resolved
modulus/utils/corrdiff/utils.py Show resolved Hide resolved
@mnabian
Copy link
Collaborator Author

mnabian commented Aug 13, 2024

/blossom-ci

@mnabian
Copy link
Collaborator Author

mnabian commented Aug 13, 2024

/blossom-ci

1 similar comment
@mnabian
Copy link
Collaborator Author

mnabian commented Aug 13, 2024

/blossom-ci

@mnabian
Copy link
Collaborator Author

mnabian commented Aug 13, 2024

/blossom-ci

@mnabian mnabian merged commit 29b5977 into NVIDIA:main Aug 13, 2024
1 check passed
@mnabian mnabian deleted the enh-ext-corrdiff_inference branch August 13, 2024 15:42
This was referenced Aug 13, 2024
Closed
Closed
@jleinonen jleinonen mentioned this pull request Sep 17, 2024
Merged
5 tasks
@mnabian mnabian mentioned this pull request Oct 17, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nbren12 nbren12 nbren12 left review comments

@jleinonen jleinonen jleinonen approved these changes

@daviddpruitt daviddpruitt Awaiting requested review from daviddpruitt

Assignees

@mnabian mnabian

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

🚀[FEA]: Modularize CorrDiff inference and move into the main package
3 participants
@mnabian @nbren12 @jleinonen