[jira] [Commented] (MATH-658) Dead code in FastMath.pow(double, double) and some improvement in test coverage
[
https://issues.apache.org/jira/browse/MATH-658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13098316#comment-13098316
]
Luc Maisonobe commented on MATH-658:
Could you provide the patch as a unified diff file with respect to current
development tree so we can review it ?
Thanks
> Dead code in FastMath.pow(double, double) and some improvement in test
> coverage
> ---
>
> Key: MATH-658
> URL: https://issues.apache.org/jira/browse/MATH-658
> Project: Commons Math
> Issue Type: Improvement
>Reporter: Yannick TANGUY
>Priority: Minor
> Fix For: 3.0
>
> Attachments: FastMathTest.java
>
>
> This issue concerns the FastMath class and its test class.
> (1) In the double pow(double, double) function, there are 2 identical "if"
> blocks. The second one can be suppressed.
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return Double.NEGATIVE_INFINITY;
> }
> // this block is never used -> to be suppressed
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> if (y > 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> (2) To obtain better code coverage, we added some tests case in
> FastMathTest.java (see attached file)
> - Added test for log1p
> - Added tests in testPowSpecialCases()
> - Added tests for a 100% coverage of acos().
> - Added tests for a 100% coverage of asin().
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-658) Dead code in FastMath.pow(double, double) and some improvement in test coverage
[
https://issues.apache.org/jira/browse/MATH-658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13100397#comment-13100397
]
Sebb commented on MATH-658:
---
The body format (3rd line onwards) is OK, but the header lines are incorrect.
They should look something like:
{code}
Index: src/main/java/org/apache/commons/math/util/FastMath.java
===
--- src/main/java/org/apache/commons/math/util/FastMath.java(revision
1166437)
+++ src/main/java/org/apache/commons/math/util/FastMath.java(working copy)
{code}
This was created by updating the file in an SVN working copy and then creating
the patch (I used Eclipse, specifying project-relative mode, but svn diff would
produce much the same output).
Your patch has completely different names and paths for the input and output
files:
{code}
--- D:/DOCUME~1/tanguyy/LOCALS~1/Temp/FastMath.java-revBASE.svn000.tmp.java
jeu. sept. 8 16:28:36 2011
+++
D:/DONNEES/ATELIER_JAVA/workspace/Commons-Math_Trunk/src/main/java/org/apache/commons/math/util/FastMath.java
jeu. sept. 8 16:10:02 2011
{code}
This means it's impossible to apply the patch automatically.
However, it's not too difficult to fix the header lines, e.g. in the above case
to:
{code}
--- FastMath.java jeu. sept. 8 16:28:36 2011
+++ FastMath.java jeu. sept. 8 16:10:02 2011
{code}
and the patch can then be applied in the appropriate directory.
No need to resubmit these particular patches, but if you submit any more please
use the proper unified diff format relative to the top-level project directory,
so paths start with src/.
> Dead code in FastMath.pow(double, double) and some improvement in test
> coverage
> ---
>
> Key: MATH-658
> URL: https://issues.apache.org/jira/browse/MATH-658
> Project: Commons Math
> Issue Type: Improvement
>Reporter: Yannick TANGUY
>Priority: Minor
> Fix For: 3.0
>
> Attachments: FastMath.java.diff, FastMathTest.java,
> FastMathTest.java.diff
>
>
> This issue concerns the FastMath class and its test class.
> (1) In the double pow(double, double) function, there are 2 identical "if"
> blocks. The second one can be suppressed.
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return Double.NEGATIVE_INFINITY;
> }
> // this block is never used -> to be suppressed
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> if (y > 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> (2) To obtain better code coverage, we added some tests case in
> FastMathTest.java (see attached file)
> - Added test for log1p
> - Added tests in testPowSpecialCases()
> - Added tests for a 100% coverage of acos().
> - Added tests for a 100% coverage of asin().
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-658) Dead code in FastMath.pow(double, double) and some improvement in test coverage
[
https://issues.apache.org/jira/browse/MATH-658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13100405#comment-13100405
]
Yannick TANGUY commented on MATH-658:
-
Ok, thanks for the tip. I'll do it next time.
> Dead code in FastMath.pow(double, double) and some improvement in test
> coverage
> ---
>
> Key: MATH-658
> URL: https://issues.apache.org/jira/browse/MATH-658
> Project: Commons Math
> Issue Type: Improvement
>Reporter: Yannick TANGUY
>Priority: Minor
> Fix For: 3.0
>
> Attachments: FastMath.java.diff, FastMathTest.java,
> FastMathTest.java.diff
>
>
> This issue concerns the FastMath class and its test class.
> (1) In the double pow(double, double) function, there are 2 identical "if"
> blocks. The second one can be suppressed.
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return Double.NEGATIVE_INFINITY;
> }
> // this block is never used -> to be suppressed
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> if (y > 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> (2) To obtain better code coverage, we added some tests case in
> FastMathTest.java (see attached file)
> - Added test for log1p
> - Added tests in testPowSpecialCases()
> - Added tests for a 100% coverage of acos().
> - Added tests for a 100% coverage of asin().
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-658) Dead code in FastMath.pow(double, double) and some improvement in test coverage
[
https://issues.apache.org/jira/browse/MATH-658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13100627#comment-13100627
]
Luc Maisonobe commented on MATH-658:
Also try to avoid static imports (and even more static imports with * at the
end).
I have fixed them.
> Dead code in FastMath.pow(double, double) and some improvement in test
> coverage
> ---
>
> Key: MATH-658
> URL: https://issues.apache.org/jira/browse/MATH-658
> Project: Commons Math
> Issue Type: Improvement
>Reporter: Yannick TANGUY
>Priority: Minor
> Fix For: 3.0
>
> Attachments: FastMath.java.diff, FastMathTest.java,
> FastMathTest.java.diff
>
>
> This issue concerns the FastMath class and its test class.
> (1) In the double pow(double, double) function, there are 2 identical "if"
> blocks. The second one can be suppressed.
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return Double.NEGATIVE_INFINITY;
> }
> // this block is never used -> to be suppressed
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> if (y > 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> (2) To obtain better code coverage, we added some tests case in
> FastMathTest.java (see attached file)
> - Added test for log1p
> - Added tests in testPowSpecialCases()
> - Added tests for a 100% coverage of acos().
> - Added tests for a 100% coverage of asin().
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-658) Dead code in FastMath.pow(double, double) and some improvement in test coverage
[
https://issues.apache.org/jira/browse/MATH-658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13100734#comment-13100734
]
Sebb commented on MATH-658:
---
Thanks - I did see the wildcard import, but left it as it is test code so not
so important.
> Dead code in FastMath.pow(double, double) and some improvement in test
> coverage
> ---
>
> Key: MATH-658
> URL: https://issues.apache.org/jira/browse/MATH-658
> Project: Commons Math
> Issue Type: Improvement
>Reporter: Yannick TANGUY
>Priority: Minor
> Fix For: 3.0
>
> Attachments: FastMath.java.diff, FastMathTest.java,
> FastMathTest.java.diff
>
>
> This issue concerns the FastMath class and its test class.
> (1) In the double pow(double, double) function, there are 2 identical "if"
> blocks. The second one can be suppressed.
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return Double.NEGATIVE_INFINITY;
> }
> // this block is never used -> to be suppressed
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> if (y > 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> (2) To obtain better code coverage, we added some tests case in
> FastMathTest.java (see attached file)
> - Added test for log1p
> - Added tests in testPowSpecialCases()
> - Added tests for a 100% coverage of acos().
> - Added tests for a 100% coverage of asin().
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-658) Dead code in FastMath.pow(double, double) and some improvement in test coverage
[
https://issues.apache.org/jira/browse/MATH-658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13100746#comment-13100746
]
Sebb commented on MATH-658:
---
Also, just noticed some tab characters in the test class patch which I have
just fixed. We don't allow tabs.
> Dead code in FastMath.pow(double, double) and some improvement in test
> coverage
> ---
>
> Key: MATH-658
> URL: https://issues.apache.org/jira/browse/MATH-658
> Project: Commons Math
> Issue Type: Improvement
>Reporter: Yannick TANGUY
>Priority: Minor
> Fix For: 3.0
>
> Attachments: FastMath.java.diff, FastMathTest.java,
> FastMathTest.java.diff
>
>
> This issue concerns the FastMath class and its test class.
> (1) In the double pow(double, double) function, there are 2 identical "if"
> blocks. The second one can be suppressed.
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return Double.NEGATIVE_INFINITY;
> }
> // this block is never used -> to be suppressed
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> if (y > 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> (2) To obtain better code coverage, we added some tests case in
> FastMathTest.java (see attached file)
> - Added test for log1p
> - Added tests in testPowSpecialCases()
> - Added tests for a 100% coverage of acos().
> - Added tests for a 100% coverage of asin().
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-658) Dead code in FastMath.pow(double, double) and some improvement in test coverage
[
https://issues.apache.org/jira/browse/MATH-658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13101186#comment-13101186
]
Yannick TANGUY commented on MATH-658:
-
Ok, here is a new patch for FastMathTest.java : I replaced the new tests and
also checked and transformed some former ones that also used exceptions.
> Dead code in FastMath.pow(double, double) and some improvement in test
> coverage
> ---
>
> Key: MATH-658
> URL: https://issues.apache.org/jira/browse/MATH-658
> Project: Commons Math
> Issue Type: Improvement
>Reporter: Yannick TANGUY
>Priority: Minor
> Fix For: 3.0
>
> Attachments: FastMath.java.diff, FastMathTest.java,
> FastMathTest.java.diff, FastMathTest.java.diff
>
>
> This issue concerns the FastMath class and its test class.
> (1) In the double pow(double, double) function, there are 2 identical "if"
> blocks. The second one can be suppressed.
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return Double.NEGATIVE_INFINITY;
> }
> // this block is never used -> to be suppressed
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> if (y > 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> (2) To obtain better code coverage, we added some tests case in
> FastMathTest.java (see attached file)
> - Added test for log1p
> - Added tests in testPowSpecialCases()
> - Added tests for a 100% coverage of acos().
> - Added tests for a 100% coverage of asin().
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-658) Dead code in FastMath.pow(double, double) and some improvement in test coverage
[
https://issues.apache.org/jira/browse/MATH-658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13101191#comment-13101191
]
Yannick TANGUY commented on MATH-658:
-
Sorry, I made the same mistake with Tortoise SVN and the header lines are still
incorrect : how do you produce a patch with Eclipse ?
-> I'll try to correct it this afternoon.
> Dead code in FastMath.pow(double, double) and some improvement in test
> coverage
> ---
>
> Key: MATH-658
> URL: https://issues.apache.org/jira/browse/MATH-658
> Project: Commons Math
> Issue Type: Improvement
>Reporter: Yannick TANGUY
>Priority: Minor
> Fix For: 3.0
>
> Attachments: FastMath.java.diff, FastMathTest.java.diff
>
>
> This issue concerns the FastMath class and its test class.
> (1) In the double pow(double, double) function, there are 2 identical "if"
> blocks. The second one can be suppressed.
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return Double.NEGATIVE_INFINITY;
> }
> // this block is never used -> to be suppressed
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> if (y > 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> (2) To obtain better code coverage, we added some tests case in
> FastMathTest.java (see attached file)
> - Added test for log1p
> - Added tests in testPowSpecialCases()
> - Added tests for a 100% coverage of acos().
> - Added tests for a 100% coverage of asin().
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-658) Dead code in FastMath.pow(double, double) and some improvement in test coverage
[
https://issues.apache.org/jira/browse/MATH-658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13101190#comment-13101190
]
Yannick TANGUY commented on MATH-658:
-
Sorry, I made the same mistake with Tortoise SVN and the header lines are still
incorrect : how do you produce a patch with Eclipse ?
-> I'll try to correct it this afternoon.
> Dead code in FastMath.pow(double, double) and some improvement in test
> coverage
> ---
>
> Key: MATH-658
> URL: https://issues.apache.org/jira/browse/MATH-658
> Project: Commons Math
> Issue Type: Improvement
>Reporter: Yannick TANGUY
>Priority: Minor
> Fix For: 3.0
>
> Attachments: FastMath.java.diff, FastMathTest.java.diff
>
>
> This issue concerns the FastMath class and its test class.
> (1) In the double pow(double, double) function, there are 2 identical "if"
> blocks. The second one can be suppressed.
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return Double.NEGATIVE_INFINITY;
> }
> // this block is never used -> to be suppressed
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> if (y > 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> (2) To obtain better code coverage, we added some tests case in
> FastMathTest.java (see attached file)
> - Added test for log1p
> - Added tests in testPowSpecialCases()
> - Added tests for a 100% coverage of acos().
> - Added tests for a 100% coverage of asin().
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-658) Dead code in FastMath.pow(double, double) and some improvement in test coverage
[
https://issues.apache.org/jira/browse/MATH-658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13101194#comment-13101194
]
Yannick TANGUY commented on MATH-658:
-
Ok, I replaced the patch and it looks like it should be.
> Dead code in FastMath.pow(double, double) and some improvement in test
> coverage
> ---
>
> Key: MATH-658
> URL: https://issues.apache.org/jira/browse/MATH-658
> Project: Commons Math
> Issue Type: Improvement
>Reporter: Yannick TANGUY
>Priority: Minor
> Fix For: 3.0
>
> Attachments: FastMath.java.diff, FastMathTest.java.diff
>
>
> This issue concerns the FastMath class and its test class.
> (1) In the double pow(double, double) function, there are 2 identical "if"
> blocks. The second one can be suppressed.
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return Double.NEGATIVE_INFINITY;
> }
> // this block is never used -> to be suppressed
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> if (y > 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> (2) To obtain better code coverage, we added some tests case in
> FastMathTest.java (see attached file)
> - Added test for log1p
> - Added tests in testPowSpecialCases()
> - Added tests for a 100% coverage of acos().
> - Added tests for a 100% coverage of asin().
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-658) Dead code in FastMath.pow(double, double) and some improvement in test coverage
[
https://issues.apache.org/jira/browse/MATH-658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13101203#comment-13101203
]
Sebb commented on MATH-658:
---
Thanks, format looks OK now.
@Luc - sorry, should have noticed the incorrect testing code.
If I'm being picky, I'd say that code such as
{code}
// Logp of -1.0 should be -Inf
Assert.assertTrue(Double.isInfinite(FastMath.log1p(-1.0)));
{code}
would be better expressed as
{code}
Assert.assertTrue("Logp of -1.0 should be
-Inf",Double.isInfinite(FastMath.log1p(-1.0)));
{code}
because it's then obvious what the error is without needing to check which line
has failed.
[And what if the test class has been amended since the test run?]
No need to resubmit; I can fix that later, but please consider for future
patches.
> Dead code in FastMath.pow(double, double) and some improvement in test
> coverage
> ---
>
> Key: MATH-658
> URL: https://issues.apache.org/jira/browse/MATH-658
> Project: Commons Math
> Issue Type: Improvement
>Reporter: Yannick TANGUY
>Priority: Minor
> Fix For: 3.0
>
> Attachments: FastMath.java.diff, FastMathTest.java.diff
>
>
> This issue concerns the FastMath class and its test class.
> (1) In the double pow(double, double) function, there are 2 identical "if"
> blocks. The second one can be suppressed.
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return Double.NEGATIVE_INFINITY;
> }
> // this block is never used -> to be suppressed
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> if (y > 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> (2) To obtain better code coverage, we added some tests case in
> FastMathTest.java (see attached file)
> - Added test for log1p
> - Added tests in testPowSpecialCases()
> - Added tests for a 100% coverage of acos().
> - Added tests for a 100% coverage of asin().
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-658) Dead code in FastMath.pow(double, double) and some improvement in test coverage
[
https://issues.apache.org/jira/browse/MATH-658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13101209#comment-13101209
]
Yannick TANGUY commented on MATH-658:
-
Ok Sebb, I'll put messages in the assertion for future patches.
> Dead code in FastMath.pow(double, double) and some improvement in test
> coverage
> ---
>
> Key: MATH-658
> URL: https://issues.apache.org/jira/browse/MATH-658
> Project: Commons Math
> Issue Type: Improvement
>Reporter: Yannick TANGUY
>Priority: Minor
> Fix For: 3.0
>
> Attachments: FastMath.java.diff, FastMathTest.java.diff
>
>
> This issue concerns the FastMath class and its test class.
> (1) In the double pow(double, double) function, there are 2 identical "if"
> blocks. The second one can be suppressed.
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return Double.NEGATIVE_INFINITY;
> }
> // this block is never used -> to be suppressed
> if (y < 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> if (y > 0 && y == yi && (yi & 1) == 1) {
> return -0.0;
> }
> (2) To obtain better code coverage, we added some tests case in
> FastMathTest.java (see attached file)
> - Added test for log1p
> - Added tests in testPowSpecialCases()
> - Added tests for a 100% coverage of acos().
> - Added tests for a 100% coverage of asin().
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
