Commons Math
  1. Commons Math
  2. MATH-1037

Distribution tests are mostly meaningless due to high tolerance

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.2
    • Fix Version/s: 3.3
    • Labels:
      None

      Description

      The tolerance used for value comparison in IntegerDistributionAbstractTest is 1E-4. However, most values being compared are much smaller, so they are considered equal even if they otherwise differ by orders of magnitude. For example, a typo in GeometricDistributionTest puts 29 in the test points instead of 19, while the test probability value is correctly given for 19. The test passes, disregarding the fact that 2.437439e-05 (test value for 19) and 1.473826e-07 (actual value for 29) differ almost hundredfold.

        Activity

        Hide
        Phil Steitz added a comment - - edited

        Thanks for reporting this. This is bad and the example calls it out. The Geometric distribution test needs to be fixed. I am inclined to get rid of the default and make getTolerance abstract so tests have to set it explicitly.

        Show
        Phil Steitz added a comment - - edited Thanks for reporting this. This is bad and the example calls it out. The Geometric distribution test needs to be fixed. I am inclined to get rid of the default and make getTolerance abstract so tests have to set it explicitly.
        Hide
        Thomas Neidhart added a comment -

        Actually, the tolerance value has been overridden in the GeometricDistributionTest, but unfortunately, the base class seems to ignore it: instead of accessing the tolerance value by calling getTolerance(), the class accesses the field directly.

        I will fix this together with the typo in the GeometricDistributionTest.

        Show
        Thomas Neidhart added a comment - Actually, the tolerance value has been overridden in the GeometricDistributionTest, but unfortunately, the base class seems to ignore it: instead of accessing the tolerance value by calling getTolerance(), the class accesses the field directly. I will fix this together with the typo in the GeometricDistributionTest.
        Hide
        Thomas Neidhart added a comment -

        Fixed in r1531413.

        Thanks for the report!

        Show
        Thomas Neidhart added a comment - Fixed in r1531413. Thanks for the report!
        Hide
        Aleksei Dievskii added a comment -

        Glad to be of help!

        Show
        Aleksei Dievskii added a comment - Glad to be of help!

          People

          • Assignee:
            Unassigned
            Reporter:
            Aleksei Dievskii
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development