Skip to content

refactor: centralize CompVisDenoiser table calculations#1383

Draft
wbruna wants to merge 1 commit intoleejet:masterfrom
wbruna:sd_refactor_sigmas
Draft

refactor: centralize CompVisDenoiser table calculations#1383
wbruna wants to merge 1 commit intoleejet:masterfrom
wbruna:sd_refactor_sigmas

Conversation

@wbruna
Copy link
Copy Markdown
Contributor

@wbruna wbruna commented Mar 31, 2026

No description provided.

Comment thread src/stable-diffusion.cpp
product *= 1.0f - powf(beta, 2.0f);
alphas_cumprod[i] = product;
}
void calculate_alphas_cumprod(float* alphas_cumprod) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've preserved this, but I'm not sure the alphas_cumprod tensor is really being used.

@wbruna wbruna force-pushed the sd_refactor_sigmas branch from 0a3b659 to e9e07bc Compare April 19, 2026 11:53
@wbruna wbruna force-pushed the sd_refactor_sigmas branch from e9e07bc to fd551b0 Compare April 19, 2026 20:03
Comment thread src/stable-diffusion.cpp
}
}

auto comp_vis_denoiser = std::dynamic_pointer_cast<CompVisDenoiser>(denoiser);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This change removes the initialization path from alphas_cumprod_tensor.

Previously, CompVisDenoiser could be initialized using the model-provided alphas_cumprod, but now it relies entirely on a hardcoded schedule.

This means we can no longer use alphas_cumprod_tensor to initialize the denoiser, which changes the original behavior.

Copy link
Copy Markdown
Contributor Author

@wbruna wbruna Apr 22, 2026

Choose a reason for hiding this comment

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

The diff doesn't show this, but the tensor is always filled by calculate_alphas_cumprod unconditionally at line ~802:

calculate_alphas_cumprod((float*)alphas_cumprod_tensor->data);

and then propagated to the comp_vis_denoiser at this point. Unless the idea is that the tensor could be overwritten by the model when loading?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Yes, in the original code path, if the model contains alphas_cumprod, it would override the default values.

@wbruna wbruna marked this pull request as draft April 22, 2026 18:12
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.

2 participants