Accumulate join Total/Avg scores in double precision

[javanna]

TermsWithScoreCollector, JoinUtil's numeric point join, and TestJoinUtil summed per-document scores in float. Float addition isn't associative, so summing the same scores in a different order can round to a different float, causing TestJoinUtil.testMultiValueRandomJoin to intermittently fail on exact score/doc-order assertions.

Scores are now accumulated in double and narrowed to float only where exposed.

[romseygeek]

LGTM.

I think this removes the only use of LongFloatHashMap in the codebase so maybe we should delete that too?

[javanna]

@romseygeek good point! I did not realize this was the only usage. I will remove as a follow-up. Thanks for the review.

[dweiss]

I understand the hope is that computing over doubles won't propagate rounding errors to the resulting float-trimmed result, right? Because in principle the same unfortunate thing can still happen with doubles, it's just less likely...

[romseygeek]

IIUC the errors will always be smaller than the delta we use in the tests if we sum over doubles though?

[javanna]

Indeed, I could no longer reproduce any failures with this fix. You still have precision errors, but when you narrow the double sum down to a float at the end ((float) total), that tiny double-level discrepancy is almost always well below the float's rounding granularity, so both orderings round to the same float.

In alternative, we could introduce some additional tolerance in the assertions?

[javanna]

In alternative, we could introduce some additional tolerance in the assertions?

The reason why I went for the current approach is that I thought it would make things a little more predictable with the introduction of search concurrency, in production code too. Let me know what you think.

[javanna]

I will merge this for now to address the test failures, let me know if there are concerns and we can always revise our approach? Thanks!

[dweiss]

I'm fine. I just wondered if switching to double precision (twice the amount of ram) is worth it, considering it's a single assertion change (adding delta). Your call.