fix: stabilize image embedding normalization - #288
Conversation
|
@emecii, thanks for taking the time to contribute. Did you hit this limitation a real model/output? Embeddings we see are small, so |
|
Thanks for checking. I tested this against the repository’s documented So I should describe this as a numerical-robustness fix rather than a currently observed image-model failure. The existing regression demonstrates the underlying arithmetic issue for finite inputs such as I only measured the default CLIP model and the bundled images, so I cannot claim this occurs with a supported production model. If that defensive scope is not useful for the project, I understand if you would prefer to close the PR. |
|
I'd prefer keeping the normalization simple if it's working fine practically. |
|
Understood — thank you for the review. Since I could not reproduce this with the documented model path and the defensive scope is not useful for the project, I am closing the PR. |
Summary
Make the shared image-embedding normalization helper stable for finite large
f32values.The previous direct squared sum overflowed for inputs such as
[f32::MAX, f32::MAX], producing[0.0, 0.0]. This scales values by their largest absolute component before accumulating the squared norm, and divides through the scaled norm so it never has to form an overflowing product.Validation
cargo fmt --all -- --checkRUSTFLAGS=-Dwarnings cargo clippycargo test --lib(16 passed)The new regression tests cover ordinary vectors, zero vectors, and finite near-maximum values. The large-value test fails against the prior implementation and passes with this change.