[jira] [Created] (MATH-834) Replace calls to the various sampling methods in "RandomDataImpl"

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

[jira] [Created] (MATH-834) Replace calls to the various sampling methods in "RandomDataImpl"

David Costanzo (Jira)
Gilles created MATH-834:
---------------------------

             Summary: Replace calls to the various sampling methods in "RandomDataImpl"
                 Key: MATH-834
                 URL: https://issues.apache.org/jira/browse/MATH-834
             Project: Commons Math
          Issue Type: Task
            Reporter: Gilles
            Assignee: Gilles
            Priority: Minor
             Fix For: 3.1


Following MATH-764 and MATH-823, the "authoritative" code that implements sampling from distributions is located in the class that represents a specific distribution (in package "o.a.c.m.distribution").
Hence, all CM code that performs sampling from a distribution should call the "sample" method from the corresponding distribution, instead of one of the methods defined in "RandomDataImpl" (in package "o.a.c.m.random").


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (MATH-834) Replace calls to the various sampling methods in "RandomDataImpl"

David Costanzo (Jira)

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

Gilles commented on MATH-834:
-----------------------------

Changes done in revision 1368738.

I tried to replace line 349 in "NaturalRanking.java" (package "o.a.c.m.stat.ranking"):
{code}
randomData.nextLong(f, f + length - 1);
{code}
with supposedly equivalent statements using "UniformRealDistribution" but a test failed.
I think that it may be due to the fact that in "sample" from "UniformRealDistribution", the upper bound is never returned, while in "nextLong" from "RandomDataImpl", the upper bound is included.
Can someone confirm?

               

> Replace calls to the various sampling methods in "RandomDataImpl"
> -----------------------------------------------------------------
>
>                 Key: MATH-834
>                 URL: https://issues.apache.org/jira/browse/MATH-834
>             Project: Commons Math
>          Issue Type: Task
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Minor
>             Fix For: 3.1
>
>
> Following MATH-764 and MATH-823, the "authoritative" code that implements sampling from distributions is located in the class that represents a specific distribution (in package "o.a.c.m.distribution").
> Hence, all CM code that performs sampling from a distribution should call the "sample" method from the corresponding distribution, instead of one of the methods defined in "RandomDataImpl" (in package "o.a.c.m.random").

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (MATH-834) Replace calls to the various sampling methods in "RandomDataImpl"

David Costanzo (Jira)
In reply to this post by David Costanzo (Jira)

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

Phil Steitz commented on MATH-834:
----------------------------------

Are these changes really necessary?  RandomData instances can be convenient. I was OK moving the implementations of the actual sampling methods to the distributions, but I see no reason that we should not keep RandomData as a means of generating random data from multiple different distributions.  It is easier to just import one class and use nextXxx methods directly than to import and instantiate multiple distribution classes in use cases (e.g. AggregateSummaryStatisticsTest) where random data from different distributions are generated.
               

> Replace calls to the various sampling methods in "RandomDataImpl"
> -----------------------------------------------------------------
>
>                 Key: MATH-834
>                 URL: https://issues.apache.org/jira/browse/MATH-834
>             Project: Commons Math
>          Issue Type: Task
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Minor
>             Fix For: 3.1
>
>
> Following MATH-764 and MATH-823, the "authoritative" code that implements sampling from distributions is located in the class that represents a specific distribution (in package "o.a.c.m.distribution").
> Hence, all CM code that performs sampling from a distribution should call the "sample" method from the corresponding distribution, instead of one of the methods defined in "RandomDataImpl" (in package "o.a.c.m.random").

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (MATH-834) Replace calls to the various sampling methods in "RandomDataImpl"

David Costanzo (Jira)
In reply to this post by David Costanzo (Jira)

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

Gilles commented on MATH-834:
-----------------------------

bq. Are these changes really necessary?

No. I thought that it would be cleaner.
It may also open the possibility to reduce the number of methods in {{RandomDataImpl}}.

bq. RandomData instances can be convenient.

I agree but IMHO, {{RandomDataImpl}} does too many things (sampling from distributions, permutations, secure RNG, secure string generation).
Maybe we could have the set of sampling utilities in a "DistributionsUtils" class, in package "distribution".

               

> Replace calls to the various sampling methods in "RandomDataImpl"
> -----------------------------------------------------------------
>
>                 Key: MATH-834
>                 URL: https://issues.apache.org/jira/browse/MATH-834
>             Project: Commons Math
>          Issue Type: Task
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Minor
>             Fix For: 3.1
>
>
> Following MATH-764 and MATH-823, the "authoritative" code that implements sampling from distributions is located in the class that represents a specific distribution (in package "o.a.c.m.distribution").
> Hence, all CM code that performs sampling from a distribution should call the "sample" method from the corresponding distribution, instead of one of the methods defined in "RandomDataImpl" (in package "o.a.c.m.random").

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Resolved] (MATH-834) Replace calls to the various sampling methods in "RandomDataImpl"

David Costanzo (Jira)
In reply to this post by David Costanzo (Jira)

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

Gilles resolved MATH-834.
-------------------------

    Resolution: Fixed

This is not a high priority issue. We can wait until the refactoring/merging of "RandomDataImpl" and "RandomData" is done and then decide what to do with the remaining occurrences of "RandomDataImpl".

               

> Replace calls to the various sampling methods in "RandomDataImpl"
> -----------------------------------------------------------------
>
>                 Key: MATH-834
>                 URL: https://issues.apache.org/jira/browse/MATH-834
>             Project: Commons Math
>          Issue Type: Task
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Minor
>             Fix For: 3.1
>
>
> Following MATH-764 and MATH-823, the "authoritative" code that implements sampling from distributions is located in the class that represents a specific distribution (in package "o.a.c.m.distribution").
> Hence, all CM code that performs sampling from a distribution should call the "sample" method from the corresponding distribution, instead of one of the methods defined in "RandomDataImpl" (in package "o.a.c.m.random").

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (MATH-834) Replace calls to the various sampling methods in "RandomDataImpl"

David Costanzo (Jira)
In reply to this post by David Costanzo (Jira)

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

Phil Steitz commented on MATH-834:
----------------------------------

The duplicated code was removed from {{RandomDataImpl}} in r1375192.
               

> Replace calls to the various sampling methods in "RandomDataImpl"
> -----------------------------------------------------------------
>
>                 Key: MATH-834
>                 URL: https://issues.apache.org/jira/browse/MATH-834
>             Project: Commons Math
>          Issue Type: Task
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Minor
>             Fix For: 3.1
>
>
> Following MATH-764 and MATH-823, the "authoritative" code that implements sampling from distributions is located in the class that represents a specific distribution (in package "o.a.c.m.distribution").
> Hence, all CM code that performs sampling from a distribution should call the "sample" method from the corresponding distribution, instead of one of the methods defined in "RandomDataImpl" (in package "o.a.c.m.random").

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira