You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "ani5rudh (via GitHub)" <gi...@apache.org> on 2023/07/22 09:04:35 UTC
[GitHub] [commons-numbers] ani5rudh opened a new pull request, #136: NUMBERS-200: Don't use compensation if it's NaN in Sum#add
ani5rudh opened a new pull request, #136:
URL: https://github.com/apache/commons-numbers/pull/136
(no comment)
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-numbers] codecov-commenter commented on pull request #136: NUMBERS-200: Don't use compensation if it's NaN in Sum#add
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #136:
URL: https://github.com/apache/commons-numbers/pull/136#issuecomment-1646575191
## [Codecov](https://app.codecov.io/gh/apache/commons-numbers/pull/136?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#136](https://app.codecov.io/gh/apache/commons-numbers/pull/136?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (c11d4c8) into [master](https://app.codecov.io/gh/apache/commons-numbers/commit/844fa9423d730136c81ee0a4f27ed63ad8cf8fba?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (844fa94) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #136 +/- ##
=========================================
Coverage 99.14% 99.14%
- Complexity 1681 1682 +1
=========================================
Files 65 65
Lines 4305 4308 +3
Branches 854 855 +1
=========================================
+ Hits 4268 4271 +3
Misses 10 10
Partials 27 27
```
| [Impacted Files](https://app.codecov.io/gh/apache/commons-numbers/pull/136?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...main/java/org/apache/commons/numbers/core/Sum.java](https://app.codecov.io/gh/apache/commons-numbers/pull/136?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29tbW9ucy1udW1iZXJzLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvbnVtYmVycy9jb3JlL1N1bS5qYXZh) | `100.00% <100.00%> (ø)` | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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: notifications-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-numbers] aherbert merged pull request #136: NUMBERS-200: Don't honour compensation if it's NaN in Sum#add
Posted by "aherbert (via GitHub)" <gi...@apache.org>.
aherbert merged PR #136:
URL: https://github.com/apache/commons-numbers/pull/136
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-numbers] aherbert commented on a diff in pull request #136: NUMBERS-200: Don't use compensation if it's NaN in Sum#add
Posted by "aherbert (via GitHub)" <gi...@apache.org>.
aherbert commented on code in PR #136:
URL: https://github.com/apache/commons-numbers/pull/136#discussion_r1271292657
##########
commons-numbers-core/src/main/java/org/apache/commons/numbers/core/Sum.java:
##########
@@ -202,7 +202,11 @@ public Sum subtract(final Sum other) {
* @return this instance.
*/
private Sum add(double s, double c) {
- return add(s).add(c);
+ add(s);
+ if (!Double.isNaN(c)) {
Review Comment:
I think this can use `isFinite(c)`. I don't think the compensation can be infinite if the regular sum is still finite, so we do not have to sum infinite compensation.
However, the NaN check should be faster so the two variants are the same. Please add a comment as to why the NaN check is required, e.g. infinite terms will generate a nan compensation. Do not destroy the regular IEEE754 sum with spurious nan.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org