Uploaded image for project: 'Commons Math'
  1. Commons Math
  2. MATH-1338

Does "EmpiricalDistribution" use "RandomDataGenerator"?

Rank to TopRank to BottomAttach filesAttach ScreenshotBulk Copy AttachmentsBulk Move AttachmentsVotersWatch issueWatchersCreate sub-taskConvert to sub-taskLinkCloneLabelsUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Bug
    • Status: Resolved
    • Blocker
    • Resolution: Fixed
    • None
    • 4.0

    Description

      The class EmpiricalDistribution (in package "o.a.c.m.random") holds an instance field ("randomData") of type RandomDataGenerator.
      The documentation of this field indicates that it is only used for sampling, and indeed the field is only accessed in order to fetch the RNG object stored in it.
      It is unclear why RandomDataGenerator was needed to hold a RNG since the same RNG could have been (in the current design) readily available in the base class.

      Sampling is performed in method getNextValue() (which in turn calls sample() in the parent class).
      getNextValue() performs a precondition check before calling sample() and raises a CM specific MathIllegalStateException; but the inherited sample() method can be still called directly on the instance, and in that case, if the (same) condition is not fulfilled, a standard NullPointerException will be thrown instead.

      In line with MATH-1158, the sampling functionality should be accessed through the RealDistribution.Sampler interface, and getNextValue() is to be removed as it duplicates the common API (i.e. the sample() method).

      Since the RNG is then passed to the createSampler factory method, the "randomData" field becomes obsolete.

      Attachments

        Issue Links

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            Unassigned Unassigned
            erans Gilles Sadowski
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment