[GitHub] [iotdb] LebronAl commented on pull request #2794: Simplify the Avg formula on removing redundant operation

[GitBox <gi...@apache.org>]

LebronAl commented on pull request #2794:
URL: https://github.com/apache/iotdb/pull/2794#issuecomment-799309216


   > > Acutally, performance is better than aggregation result's accuracy in our user's recent feedbacks. So we had better create a new datatype if there are really some user cases caring about the accuracy very much in the future.
   > > BTW, I do see something inappropriate with the current code in this PR. Maybe you can fix it in this PR, there are two main problems:
   > > 
   > > 1. some redundant operation such as `((double) statistics.getCount() / cnt) * sum / statistics.getCount();`
   > > 2. the test should be performed as a delta way, such as if `|a - 11.93333| < 0.000001` then test passed or test failed.
   > > 
   > > What's your opinion?
   > 
   > Hello @LebronAl,
   > Thanks for your reply, I am confused about what's your meaning of "performance is better than aggregation result's accuracy"?
   > If we do not use Bigdecimal way, it is a good idea only to solve this issue by simplifing and using delta test value. However, why don't we use Bigdecimal to calculate the avg which also can guarantee the accuracy instead of creating a new datatype?
   
   Sorry, I missed a key sentence in last reviews. 
   > BigDecimal maybe a significant performance penalty.
   
   As you can benchmark the performance on your own or search out some [results](https://www.iteye.com/problems/63215),
   There is no need to compromise performance for accuracy issues that users do not respond to.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org