Improve averageBigInteger and sumBigInteger precision/conversion #286
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
simonbasle
added
area/reactor-extra
|
cc @Sunshow, I actually discovered a few drawbacks with your original approach and went a slightly different direction. |
|
awesome. |
|
@simonbasle this PR seems to have been merged on a maintenance branch, please ensure the change is merge-forwarded to intermediate maintenance branches and up to |
simonbasle
added a commit
that referenced
this pull request
Apr 11, 2022
simonbasle
added a commit
to reactor/reactor-kotlin-extensions
that referenced
this pull request
Jun 17, 2022
This commit adapts to changes in precision in addons' sumBigInteger that were introduced in the previously referenced snapshot (3.4.8). It also upgrades the dependency to reflect the latest core and addons snapshots, as the project was previously out of sync. For the change in sumBigInteger, see reactor/reactor-addons#286: Now the loss of precision is only triggered at the end of the sum, avoiding lossy conversion of each floating point number that is being summed.
simonbasle
added a commit
to reactor/reactor-kotlin-extensions
that referenced
this pull request
Jun 17, 2022
This commit adapts to changes in precision in addons' sumBigInteger that were introduced in the previously referenced snapshot (3.4.8). It also upgrades the dependency to reflect the latest core and addons snapshots, as the project was previously out of sync. For the change in sumBigInteger, see reactor/reactor-addons#286: Now the loss of precision is only triggered at the end of the sum, avoiding lossy conversion of each floating point number that is being summed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit improves the precision and conversion behavior of
MathFluxsumBigIntegerandaverageBigIntegermethods. These method better deal withfractional types by internally summing using a
BigDecimalrather than aBigInteger. As a result, the only lossy step is at the end when the result iseffectively presented as a
BigInteger:sum: remaining fractional part of the BigDecimal sum is droppedaverage: the division bycountis applied, then the result's fractionalpart is dropped
Previously the fractional part would be progressively dropped by implicit
conversion of each incoming element to
long.Similar to precision improvement done in #260/#261 and earlier iteration done
in #284.