EDM Diffusion Models

[mnabian]

Modulus Pull Request

Description

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]

/blossom-ci

[mnabian]

/blossom-ci

[mnabian]

/blossom-ci

[mnabian]

/blossom-ci

[mnabian]

/blossom-ci

[mnabian]

/blossom-ci

[mnabian]

/blossom-ci

[mnabian]

/blossom-ci

[mnabian]

/blossom-ci

[dallasfoster]

There are a lot of places where documentation can be improved and notation is not entirely explain.

The examples folder should at least include a readme with an explanation about how to use the example.

The placement of the layers and folders should be reconsidered and generalized wherever possible.

Creation of a generic, modular, langevin diffusion class should be considered as a separate epic.

There are several places with large chunks of commented out code, please consider uncommenting or removing these pieces of code.

Please let me know which of these and the other comments you think should be in-scope for this PR.

[mnabian]

Thanks @dallasfoster for reviewing this PR!

There are a lot of places where documentation can be improved and notation is not entirely explain.

I agree. I added more documentation to loss.py, and I will keep improving the documentation in the subsequent PRs.

The examples folder should at least include a readme with an explanation about how to use the example.

The example is a work in progress. Once the example is ready, I will also include a README with detailed instructions.

The placement of the layers and folders should be reconsidered and generalized wherever possible.

I agree. I have added this to the list of issues in this epic: #196. Will decide about this once I develop the example and get a better understanding of what can be generalized.

Creation of a generic, modular, langevin diffusion class should be considered as a separate epic.

For now I added it to this epic: #196

There are several places with large chunks of commented out code, please consider uncommenting or removing these pieces of code.

The commented out code is mostly the ResDiff code and it helps me generalize the existing code to add support for ResDiff and super-resolution task. I propose we keep them in this PR, and I can remove the remaining comments right before the release.

[mnabian]

/blossom-ci

[loliverhennigh]

Hey @mnabian, to be honest I struggled a bit with giving this a review. I ran through all the code. There is quite a lot of detail with how then diffusion models work and I am not very familiar with all the variations in this space. Certainty a lot more complex then just basic regression like many of our examples.

At this point I don't really have any good advice about integrating this better with modulus other then some of the comments Dallas made such as moving over some of the layer functions over. I think we should continue to treat this as a long term project following your epic, #196. Lets just get this in and as the examples build I think we will be able to work out abstractions that make sense...

[mnabian]

/blossom-ci

[dallasfoster]

Thank you for adding the necessary tasks to the epic list. I also appreciate the additional documentation. I think at some point we would want to include the examples in the ruff linting, but I understand that might be a separate PR after the launch examples have been moved. Everything else here looks manageable.