zhangstar333 commented on PR #67039: URL: https://github.com/apache/doris/pull/67039#issuecomment-5421505500
> Thanks — that log made it clear, and the guest login works, so I can read TeamCity myself from now on. > > The cause is narrower than the digit drift I guessed at. Cosine distance is `1 - cos_sim`, and the cosine of a row against itself is mathematically exactly 1, so the distance is 0. It does not come out that way for `float16`: the query literal is the value _before_ half-precision rounding while the column stores the value _after_, so the two are not bit-identical, `cos_sim` lands on `1 ± 1ulp`, and the distance is `±2^-23` — which is exactly the `1.1920929E-7` in the golden. Which sign it takes depends on the accumulation order of the build running the query. > > That is why the framework's 1e-6 relative tolerance did not absorb it: the rows below it passed, but relative error is meaningless around zero, and `+eps` against `-eps` comes out as 2.0. > > Two things confirm the diagnosis: > > * **Float32 cosine is unaffected.** `test_lance_vector_search_metrics` runs the same metric over the same directional data and records a clean `0.0` self-distance, and it passed on this run. There the query literal and the stored value are bit-identical, so `cos_sim` really is 1. > * **Float16 L2 is unaffected.** `(x - x)^2` is 0 whatever the rounding; only a metric that divides by the vector norms introduces the ulp. > > ### Fix > The Float16 table is the one table in this suite whose data has no closed-form expected distance — Float64 records the L2 ladder `0, 16, 64, 144, 256` and UInt8 the hamming ladder `0, 1, 2, 3, 4`, both exact integers on any platform. So for Float16 the distance column was never recording a checkable value, only whatever the platform computed. Its two blocks now record `row_id, label` and the row order, which is the part that carries meaning. Float64 and UInt8 keep their distances. > > The magnitude is still worth pinning — a backend that widened Float16 to Float32 before comparing, or lost the query vector's precision, would move it off one ulp — so it is now an assertion on the absolute value with a tolerance, rather than a literal whose sign a golden cannot avoid recording: > > ```groovy > assertTrue(Math.abs(f16SelfDistance) < 1e-5, ...) > ``` > > I also checked the rest of `external_table_p0/lance` for the same shape: this was the only near-zero float literal in any golden there, and `f16_cosine_flat` — added in this PR, and never reached on CI because the suite aborts at the first failure — had the identical exposure. Both are fixed. > > Full `external_table_p0/lance` run locally on the new head: 13/14. The one failure is `test_lance_show_index`, which needs the FE-side Lance index metadata from #66637; the FE I have running locally predates it, and CI builds the FE from this branch, which does contain it. get it. let's test again in TeamCity. @FANNG1 and the TeamCity could triggered by comment, Comment `run buildall` on your PR to trigger all TeamCity pipelines. Comment `/review` on your PR to trigger AI 'code-review' -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
