Skip to content

Fix EDM DPM Solver Test and Enhance Test Coverage #11679

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
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix EDM DPM Solver Test and Enhance Test Coverage #11679

wants to merge 1 commit into from

Conversation

@mohiuddin-khan-shiam
Copy link

@mohiuddin-khan-shiam mohiuddin-khan-shiam commented Jun 8, 2025

Changes

  • Fixed the previously skipped test_solver_order_and_type in test_scheduler_edm_dpmsolver_multistep.py
  • Properly separated testing of dpmsolver++ and sde-dpmsolver++ algorithms
  • Added comprehensive validation for all supported solver orders and types
  • Included output shape verification in test assertions
  • Improved error messages for better debugging

Testing

  • Verified all tests pass for both algorithm types
  • Confirmed proper handling of solver order constraints
  • Validated output shapes match expected dimensions

Impact

  • Improves test reliability for the EDM DPM Solver
  • Enhances maintainability with better test organization
  • Provides clearer feedback when tests fail

@mohiuddin-khan-shiam
Fix EDM DPM Solver Test and Enhance Test Coverage
7bec815 
This PR resolves the previously skipped test_solver_order_and_type in the EDM DPM Solver tests by properly handling both dpmsolver++ and sde-dpmsolver++ algorithm types. The test now includes comprehensive validation for all supported solver orders and types, with improved error messages and additional assertions to verify output shapes. The changes ensure better test reliability and maintainability while providing clearer feedback for debugging.
@yiyixuxu
Copy link
Collaborator

yiyixuxu commented Jun 12, 2025

can you take a look here? @DN6 @sayakpaul

sayakpaul
sayakpaul reviewed Jun 13, 2025
View reviewed changes
tests/pipelines/stable_diffusion_adapter/test_stable_diffusion_adapter.py
expected_max_diff=2e-3,
additional_params_copy_to_batched_inputs=["num_inference_steps"],
):
# Set default test behavior based on device
Copy link
Member

@sayakpaul sayakpaul Jun 13, 2025

Choose a reason for hiding this comment

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

(nit): indentation?

sayakpaul
sayakpaul reviewed Jun 13, 2025
View reviewed changes
Copy link
Member

@sayakpaul sayakpaul left a comment

Choose a reason for hiding this comment

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

Thanks for your PR. Can we break this into smaller PRs? For example, the scheduler related changes go sit in one, and the pipelines-related ones could sit in another.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sayakpaul sayakpaul sayakpaul left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mohiuddin-khan-shiam @yiyixuxu @sayakpaul