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

Gilles commented on MATH-1370:
------------------------------

Thanks for confirming.

bq. It is also testing the getPmf() function which is specific to 
EnumeratedDistribution

IIUC {{getPmf}} is indirectly tested in tests for 
{{EnumeratedIntegerDistribution}} and {{EnumeratedRealDistribution}} since it 
is called from several methods in those classes.

It does not hurt to have additional tests but it is clearer to limit their 
scope to a single purpose.

I've created branch "feature-MATH-1370".  Please have a look and let me know 
whether everything is OK for you.


> Increase efficiency of EnumeratedDistribution#probability
> ---------------------------------------------------------
>
>                 Key: MATH-1370
>                 URL: https://issues.apache.org/jira/browse/MATH-1370
>             Project: Commons Math
>          Issue Type: Improvement
>            Reporter: Ryan Gaffney
>            Priority: Minor
>              Labels: patch, performance
>         Attachments: enum-distribution-perf-patch, 
> enum-distribution-perf-patch.2
>
>
> There are lots of other low hanging fruit in the distribution package but 
> unfortunately this is the only one I got to that day.
> In the EnumeratedDistribution case, the probability() method is currently 
> O(N) where N denotes number of random variables, even though the random 
> variables are fixed / known. This change makes probability() O(1). The 
> unlikely worst case scenario now consumes slightly more than 2x the memory, 
> but I do not think this would be an issue in the vast majority of cases.
> Original PR (incorrectly against master) 
> [here|https://github.com/apache/commons-math/pull/34].



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to