[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