Skip to content

Add ddpm model#56

Merged
jemrobinson merged 49 commits intomainfrom
add-ddpm-model
Aug 28, 2025
Merged

Add ddpm model#56
jemrobinson merged 49 commits intomainfrom
add-ddpm-model

Conversation

@marianovitasari20
Copy link
Copy Markdown
Contributor

@marianovitasari20 marianovitasari20 commented Aug 14, 2025

Closes #55

Comment thread ice_station_zebra/models/diffusion/gaussian_diffusion.py Outdated
Comment thread ice_station_zebra/models/diffusion/gaussian_diffusion.py Outdated
Comment thread ice_station_zebra/models/diffusion/gaussian_diffusion.py Outdated
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Aug 26, 2025

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  ice_station_zebra/models/common
  __init__.py
  timeembed.py 26-30, 37
  ice_station_zebra/models/diffusion
  __init__.py
  gaussian_diffusion.py 41-74, 93-98, 117-137, 153-156, 177-182
  unet_diffusion.py 49-102, 123-173, 189-204, 217-220
  ice_station_zebra/models/processors
  __init__.py
  ddpm.py 24-28, 40-46, 64-80
  ice_station_zebra/types
  __init__.py
  complex_datatypes.py 37, 52-59, 63-65, 69
  enums.py
  simple_datatypes.py
  typedefs.py
Project Total  

This report was generated by python-coverage-comment-action

@marianovitasari20 marianovitasari20 requested a review from a team August 27, 2025 12:31
@jemrobinson
Copy link
Copy Markdown
Member

I can review this today/tomorrow.

Copy link
Copy Markdown
Member

@jemrobinson jemrobinson left a comment

Choose a reason for hiding this comment

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

We should use B, C, H, W format throughout the diffusion process. Happy to help with implementing this if it's not clear how to do it.

We also don't need EMA to be implemented inside the model, as we're running EMA in a lightning callback.

Comment thread ice_station_zebra/models/common/timeembed.py
Comment thread ice_station_zebra/models/diffusion/gaussian_diffusion.py Outdated
Comment thread ice_station_zebra/models/diffusion/gaussian_diffusion.py
Comment thread ice_station_zebra/models/diffusion/gaussian_diffusion.py
Comment thread ice_station_zebra/models/diffusion/unet_diffusion.py Outdated
Comment thread ice_station_zebra/models/diffusion/unet_diffusion.py
Comment thread ice_station_zebra/models/diffusion/unet_diffusion.py
Comment thread ice_station_zebra/models/processors/ddpm.py
Comment thread pyproject.toml
Comment thread ice_station_zebra/models/diffusion/gaussian_diffusion.py
@jemrobinson
Copy link
Copy Markdown
Member

Also, can you add some tests for the model? If you merge main into this branch, you can see some examples in tests/models/test_processors.py.

@marianovitasari20
Copy link
Copy Markdown
Contributor Author

Also, can you add some tests for the model? If you merge main into this branch, you can see some examples in tests/models/test_processors.py.

#83

@marianovitasari20
Copy link
Copy Markdown
Contributor Author

Thank you so much, @jemrobinson, for the useful feedback. I will adjust them soon.

Copy link
Copy Markdown
Member

@jemrobinson jemrobinson left a comment

Choose a reason for hiding this comment

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

LGTM

@jemrobinson jemrobinson merged commit 9b98709 into main Aug 28, 2025
3 checks passed
@jemrobinson jemrobinson deleted the add-ddpm-model branch August 28, 2025 21:25
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.

Integrate Maria's Model (DDPM) into main branch

3 participants