stojkomilos commented on PR #515: URL: https://github.com/apache/datasketches-cpp/pull/515#issuecomment-5477476809
reply to @c-dickens `Could you explain more about the intended use-case? I think trim() + compact(true) already produces this sketch (your own test asserts the output matches it on theta and retained set) and doesn't require an api change?` - As I mentioned, trim() + compact() is slow, as it does allocation of new memory and it's slower than the current implementation for other reasons also `Unsure we need union parity here because union clips its result to k hashes as its accumulating state has to stay bounded across arbitrarily many merges. An update sketch does not have this constraint. It holds up to ~15/16(2k) values between rebuilds, and compact() keeps all of them because relative error goes as 1/√(retained), not 1/√k — so trimming discards up to half the values and widens the bound.` - sure, we don't need parity, then we can rename the function from getResult (or get_result) to getCompactTrimmed() -- 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]
