[jira] Created: (MATH-183) Findbugs Report

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

[jira] Created: (MATH-183) Findbugs Report

JIRA jira@apache.org
Findbugs Report
---------------

                 Key: MATH-183
                 URL: https://issues.apache.org/jira/browse/MATH-183
             Project: Commons Math
          Issue Type: Bug
            Reporter: Sebb


The attachment (to follow) is a summary of a Findbugs run.

It's possible that the use of == for comparing floats is intended; if so perhaps it should be commented.

I think all the other bug reports are valid, though of course the ones relating to the exposure of internal implementation may be ignored

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (MATH-183) Findbugs Report

JIRA jira@apache.org

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

Sebb updated MATH-183:
----------------------

    Attachment: MathFindbugs.csv

CSV file summarising Findbugs report

> Findbugs Report
> ---------------
>
>                 Key: MATH-183
>                 URL: https://issues.apache.org/jira/browse/MATH-183
>             Project: Commons Math
>          Issue Type: Bug
>            Reporter: Sebb
>         Attachments: MathFindbugs.csv
>
>
> The attachment (to follow) is a summary of a Findbugs run.
> It's possible that the use of == for comparing floats is intended; if so perhaps it should be commented.
> I think all the other bug reports are valid, though of course the ones relating to the exposure of internal implementation may be ignored

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (MATH-183) Findbugs Report

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/MATH-183?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12564978#action_12564978 ]

Luc Maisonobe commented on MATH-183:
------------------------------------

The first two warnings about the non final TOO_SMALL fields have been discussed on the list (see http://markmail.org/message/tnxlul6qfjzs3mr3). Phil proposed to comment this in the release notes.

The next four floating point equality warnings and the last four internal representation exposed are intentional. They are identified in a commented findbugs exclusion filter in the subversion tree (file findbugs-exclude-filter.xml added about three weeks ago). This filter is active when findbugs report is created from maven2 (see plugin configuration in pom.xml).

The null pointer possible dereference was also discussed here (see http://markmail.org/message/33jpeig4qjmvtvy7). I forgot to check after Phil answered. I'll do it.

I'll fix the static inner class in Frequency if possible without breaking compatibility.

I understand the concerns about synchronisation, but would prefer someone else check them also.

As a summary, only the first two warnings should remain at the end.
Thanks for the report.


> Findbugs Report
> ---------------
>
>                 Key: MATH-183
>                 URL: https://issues.apache.org/jira/browse/MATH-183
>             Project: Commons Math
>          Issue Type: Bug
>            Reporter: Sebb
>         Attachments: MathFindbugs.csv
>
>
> The attachment (to follow) is a summary of a Findbugs run.
> It's possible that the use of == for comparing floats is intended; if so perhaps it should be commented.
> I think all the other bug reports are valid, though of course the ones relating to the exposure of internal implementation may be ignored

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (MATH-183) Findbugs Report

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/MATH-183?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12564982#action_12564982 ]

Sebb commented on MATH-183:
---------------------------

OK.

However, I suggest that the floating point equality tests should be commented in the actual code, as it's an unusual idiom.

As to synchronisation, it depends partly on whether the Math classes are intended to be thread-safe or not.

> Findbugs Report
> ---------------
>
>                 Key: MATH-183
>                 URL: https://issues.apache.org/jira/browse/MATH-183
>             Project: Commons Math
>          Issue Type: Bug
>            Reporter: Sebb
>         Attachments: MathFindbugs.csv
>
>
> The attachment (to follow) is a summary of a Findbugs run.
> It's possible that the use of == for comparing floats is intended; if so perhaps it should be commented.
> I think all the other bug reports are valid, though of course the ones relating to the exposure of internal implementation may be ignored

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (MATH-183) Findbugs Report

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/MATH-183?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12565203#action_12565203 ]

Luc Maisonobe commented on MATH-183:
------------------------------------

I have looked again at the synchronisation issues.
The one in ResizableDoubleArray was straightforward and fixed.
The ones in SummaryStatistics are more difficult. They are due to the various setters and getters for implementations (set*Impl and get*Impl) which are synchronized, whereas other methods using these implementations are not synchronized.  In fact, there are more issues than the two appearing in the report: if we fix these two, a bunch of other problems appear, which should also be fixed. At the end, almost all methods end up being synchronized. This seems not to be the objective of this class but rather the objective of the SynchronizedSummaryStatistics.
So I would suggest moving the synchronization on the setters/getters from the SummaryStatistics class to the SynchronizedSummaryStatistics. derived class.
What do you think ?

> Findbugs Report
> ---------------
>
>                 Key: MATH-183
>                 URL: https://issues.apache.org/jira/browse/MATH-183
>             Project: Commons Math
>          Issue Type: Bug
>            Reporter: Sebb
>         Attachments: MathFindbugs.csv
>
>
> The attachment (to follow) is a summary of a Findbugs run.
> It's possible that the use of == for comparing floats is intended; if so perhaps it should be commented.
> I think all the other bug reports are valid, though of course the ones relating to the exposure of internal implementation may be ignored

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (MATH-183) Findbugs Report

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/MATH-183?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12565206#action_12565206 ]

Sebb commented on MATH-183:
---------------------------

Yes, making SynchronizedSummaryStatistics fully sync, and removing synch from SummaryStatistics seems like a good idea, as the latter class is neither one thing nor the other.

> Findbugs Report
> ---------------
>
>                 Key: MATH-183
>                 URL: https://issues.apache.org/jira/browse/MATH-183
>             Project: Commons Math
>          Issue Type: Bug
>            Reporter: Sebb
>         Attachments: MathFindbugs.csv
>
>
> The attachment (to follow) is a summary of a Findbugs run.
> It's possible that the use of == for comparing floats is intended; if so perhaps it should be commented.
> I think all the other bug reports are valid, though of course the ones relating to the exposure of internal implementation may be ignored

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (MATH-183) Findbugs Report

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/MATH-183?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12565208#action_12565208 ]

Phil Steitz commented on MATH-183:
----------------------------------

I agree that moving synch to the synch classes is better.

> Findbugs Report
> ---------------
>
>                 Key: MATH-183
>                 URL: https://issues.apache.org/jira/browse/MATH-183
>             Project: Commons Math
>          Issue Type: Bug
>            Reporter: Sebb
>         Attachments: MathFindbugs.csv
>
>
> The attachment (to follow) is a summary of a Findbugs run.
> It's possible that the use of == for comparing floats is intended; if so perhaps it should be commented.
> I think all the other bug reports are valid, though of course the ones relating to the exposure of internal implementation may be ignored

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Resolved: (MATH-183) Findbugs Report

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

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

Luc Maisonobe resolved MATH-183.
--------------------------------

       Resolution: Fixed
    Fix Version/s: 1.2

synchronized setters/getters have been moved to SynchronizedSummaryStatistics class as of r618097

> Findbugs Report
> ---------------
>
>                 Key: MATH-183
>                 URL: https://issues.apache.org/jira/browse/MATH-183
>             Project: Commons Math
>          Issue Type: Bug
>            Reporter: Sebb
>             Fix For: 1.2
>
>         Attachments: MathFindbugs.csv
>
>
> The attachment (to follow) is a summary of a Findbugs run.
> It's possible that the use of == for comparing floats is intended; if so perhaps it should be commented.
> I think all the other bug reports are valid, though of course the ones relating to the exposure of internal implementation may be ignored

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.