fix: address Ideogram review feedback - #20
Conversation
Support Ideogram linear weight_scale tensors and avoid staging inactive cond/uncond transformer params during non-segmented offload. Co-authored-by: Cursor <cursoragent@cursor.com>
Fail Ideogram generation requests that need CFG when the standalone unconditional diffusion model was not successfully loaded. Co-authored-by: Cursor <cursoragent@cursor.com>
| if (weight_scale->ne[0] == x->ne[0] && ggml_n_dims(weight_scale) == 1) { | ||
| weight_scale = ggml_reshape_3d(ctx->ggml_ctx, weight_scale, weight_scale->ne[0], 1, 1); | ||
| } | ||
| return ggml_mul(ctx->ggml_ctx, x, weight_scale); |
There was a problem hiding this comment.
This still applies weight_scale in the wrong order for biased linear layers. Linear::forward() has already added the bias, so this returns (xW + b) * weight_scale. For FP8 scaled weights the scale belongs to the weight product, not the bias; the expected computation is xW * weight_scale + b.
This matters for Ideogram because many of these linears have bias=true (input_proj, llm_cond_proj, timestep/adalan projections, final linear, etc.), so official FP8-with-scale checkpoints will still produce incorrect activations even though the scale tensor is now loaded. Please either teach Linear to handle weight_scale before bias, or have Ideogram4Linear call the underlying matmul without bias, multiply by weight_scale, then add bias afterward.
FP8 weight_scale belongs to the weight product, so compute the matmul without bias, multiply by weight_scale, then add bias: (xW)*scale + b instead of (xW + b)*scale. Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
weight_scaletensors for linear layers so FP8-with-scale checkpoints are not silently run without their scales.Test plan
git diff --checkcmake -S . -B build-ideogram-review -DSD_BUILD_EXAMPLES=OFF -DSD_BUILD_SHARED_LIBS=OFF -DGGML_NATIVE=OFFcmake --build build-ideogram-review --config Release -j 8Made with Cursor