Re: BUG #15307: Low numerical precision of (Co-) Variance
От | Madeleine Thompson |
---|---|
Тема | Re: BUG #15307: Low numerical precision of (Co-) Variance |
Дата | |
Msg-id | 153802514313.25845.6696084477831363882.pgcf@coridan.postgresql.org обсуждение исходный текст |
Ответы |
Re: BUG #15307: Low numerical precision of (Co-) Variance
Re: BUG #15307: Low numerical precision of (Co-) Variance |
Список | pgsql-hackers |
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: not tested Documentation: not tested This is my first PostgreSQL review. Hopefully I'm getting it right. I independently ran into the bug in question and foundthat the author had already written a patch. I'm happy the author wrote this. (1) I am not experienced with PostgreSQL internals, so I can't comment on the coding style or usage of internal functions. (2) The patch applies cleanly to ce4887bd025b95c7b455fefd817a418844c6aad3. "make check", "make check-world", and "make install"pass. (3) The patch addresses the numeric inaccuracy reported in the bug. (Yay!) I believe the tests are sufficient to confirmthat it works as intended. I don't think the test coverage *before* this patch was sufficient, and I appreciate theimproved test coverage of the combine functions. I verified the new tests manually. (4) The comments cite Youngs & Cramer (1971). This is a dated citation. It justifies its algorithms based on pre-IEEE754single-precision arithmetic. Most notably, it assumes that floating-point division is not exact. That said, thealgorithm is still a good choice; it is justified now because compared to the standard Welford variance, it is less proneto causing pipeline stalls on a modern CPU. Maybe link to Schubert & Gentz 2018 (https://dl.acm.org/citation.cfm?id=3223036)instead. The new float8_combine combine code is (almost) S&G eqn. 22; the float8_accumcode is S&G eqn. 28. float8_regr_accum and float8_regr_combine are S&G eqn. 22. (5) I think the comment /* Watch out for roundoff error producing a negative variance */ and associated check are obsoletenow. The new status of this patch is: Waiting on Author
В списке pgsql-hackers по дате отправления: