[jira] [Created] (MATH-658) Dead code in FastMath.pow(double, double) and some improvement in test coverage

classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (MATH-658) Dead code in FastMath.pow(double, double) and some improvement in test coverage

ASF GitHub Bot (Jira)

    [ 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

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (MATH-658) Dead code in FastMath.pow(double, double) and some improvement in test coverage

ASF GitHub Bot (Jira)
In reply to this post by ASF GitHub Bot (Jira)

    [ 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

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (MATH-658) Dead code in FastMath.pow(double, double) and some improvement in test coverage

ASF GitHub Bot (Jira)
In reply to this post by ASF GitHub Bot (Jira)

    [ 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

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Resolved] (MATH-658) Dead code in FastMath.pow(double, double) and some improvement in test coverage

ASF GitHub Bot (Jira)
In reply to this post by ASF GitHub Bot (Jira)

     [ https://issues.apache.org/jira/browse/MATH-658?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sebb resolved MATH-658.
-----------------------

    Resolution: Fixed

Hope this is now better resolved ...

> 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

       
12