Re: [RNG] About RNG-6 (Was: [VOTE][RC1] Release Commons Rng 1.0)
Le 15/09/2016 à 16:37, Gilles a écrit : > I don't understand "motivations for the API". I just mean a short introduction to the component, something explaining why it exists, its usefulness (performance, randomness quality), the implementations supported, etc. > I won't do that. > It was already a pain to create those tables as they are (see the > doc source). I understand you don't want to do it, editing apt files isn't fun. But are you opposed to the idea? >> - The internal packages should be hidden from the Javadoc, > > I do not know how do that. Packages can be excluded from the Javadoc by configuring the maven-javadoc-plugin with the element: https://maven.apache.org/plugins/maven-javadoc-plugin/examples/exclude-package-names.html > And I don't see the point, since from the "binary" POV, they > are "public". That's purely from the documentation point of view here. If we don't want users to use these packages, let's not mention them in the Javadoc. That's an extra security with the "internal" package name. > Users can select a generator without knowing the gory details. > If they want to know more, they go to the internals. > It they really want to know, they go to the references. > > Internal should not be hidden: contributors who'd fancy to provide > additional generators do not have to go through any hoops. > > Or do you suggest that this component should supply two JARs: > * commons-rng-api > Contents of "o.a.c.rng" > * commons-rng-core > Contents of "o.a.c.rng.internal" > ? > > And two sets of Javadoc? No thanks, too complicated. Contributors will just checkout the code and quickly figure out how it's organized and how it's meant to be extended, I don't worry about that. But I worry about mere users getting confused about the API. >> - Why isn't the UniformRandomProvider interface simply named >> RandomProvider? > > Maybe because we want to be transparent on what the code does (?). > > With this name, there is no ambiguity: no more, no less than what > the name implies. I don't feel there is an ambiguity here, probably because I'm not an expert in random number generators. I just see an excessively long class name that could be shortened. CM4/Hipparchus just use 'RandomGenerator' for this interface, that's in line with the name of the component and a better choice in my opinion. >> - UniformRandomProvider mimics the java.util.Random methods which is >> nice. Is there any value in also implementing the nextGaussian() method >> such that our API is a perfect drop-in replacement for the JDK class? > > No. [IMHO.] > This was discussed on this list (and cf. above point). That would be convenient for migrating code using java.util.Random to commons-rng though. CM has it and the implementation doesn't look so complicated, I'm not sure to understand why you think it should go elsewhere. Even if more elaborated implementations could be latter imagined there is no harm providing a default one as a convenience for migrating legacy code. >> - For code relying on the java.util.Random type, it might be useful to >> provide an adapter turning an UniformRandomProvider into a >> java.util.Random implementation (Hipparchus has one). > > This was discussed on this list. > > And see Commons Math's classes "o.a.c.math4.random.RandomUtils" and > "o.a.c.math4.random.RngAdaptor" classes. > > In the latter (being deprecated as of its creation), I indicated why > it is a bad idea. > > Again, if we really want to support the use-case you proposed, it's > a utility that should go in another component. The component already converts a java.util.Random into our RandomProvider, I tend to think it's obvious to support the reverse operation in the same component. >> - The choice of an enum for the factory class RandomSource is a bit >> unusual. It is probably ok for simple RNG with a simple seed, but with >> more complex implementations I find it less intuitive to use. > > Less intuitive than what? Less intuitive than other designs, such as the one I proposed. >> When a user types this in its IDE, there is no type information nor >> contextual documentation for the parameters expected. The user has to >> know the type of the seed and the meaning of the parameters expected, >> the IDE won't be able to help here (with CTRL+P and CTRL+Q in IntelliJ). > > Honestly, I don't care. > To put it less bluntly, the choice of RNG to instantiate is not a > matter of IDE. I don't think you understand my point. I don't expect the IDE to pick the right RNG for my use case of course, that's impossible. But I expect the IDE to help me using the API by hinting the type expected and providing contextual documentation about the method used. The current design makes this impossible. The users are forced to open the Javadoc in their browser when they want to instantiate a generator they don't know or remember about. I don't think it's a good idea. > Practically, the lack of type information (w
Re: [RNG] About RNG-6 (Was: [VOTE][RC1] Release Commons Rng 1.0)
After scanning this, I am wondering if a smoother path to 1.0 would be really be through a beta or alpha release. The code is all there, any one can look at it. The advantage of a beta release is that it gets pushed out as a ANNOUNCE email and might get more notice. YMMV. Gary On Thu, Sep 15, 2016 at 7:37 AM, Gilles wrote: > Hi. > > Moving to another thread, as your comments do not belong in > a release vote. > > Many of them could lead to interesting discussions that should > (and could) have happened earlier. > > Indeed, for the record, see > https://issues.apache.org/jira/browse/RNG-6 > and the numerous posts which I sent to this list > * while ironing out the code of this new component in its own >repository, and doing the various chores (site, userguide, >quality testing, Github, Travis, Coveralls, etc) for the last >6 weeks, > * while developing that code in a new Commons Math package >"o.a.c.math4.rng" for the last 6 months, > * while fixing the code in Commons Math "o.a.c.m.random" >package for the last 9 months. > > For a number of issues which you now raise, I've explicitly > asked for input. > > I find it a bit strange that, at this precise point, you start > to question the choices which I've made in the absence of any > prior remarks, suggestions, or warnings. > > On Wed, 14 Sep 2016 17:04:53 +0200, Emmanuel Bourg wrote: > >> Hi all, >> >> RNG is moving fast, that's nice. I got a quick look at the API and the >> site, here are my comments: >> >> - The main page (Overview) could be enhanced with a description of the >> API longer than a single sentence. It could mention the motivations for >> the API, the implementations supported, a sample code snippet and a link >> to the user guide. >> > > I don't understand "motivations for the API". > > Do you mean something like my comment in > https://issues.apache.org/jira/browse/RNG-6 > ? > > [My motivation for proposing this code can be gathered from > the archives and the bugs and shortcomings reported in the > JIRA MATH project, and related posts here). They do not > belong in that "overview" page.] > > - In the Performance section of the User Guide, I'd suggest grouping the >> 3 tables into a single one. It might also be interesting to put the >> quality results in the same table, so we can see the trade-off between >> performance and quality. >> > > I won't do that. > It was already a pain to create those tables as they are (see the > doc source). > > - The License section of the user guide could be removed, I guess >> everyone knows that Apache projects use the Apache license. >> > > No problem. > I thought that it was mandatory. > > All the pages were copied from the corresponding Commons Math ones. > > - The internal packages should be hidden from the Javadoc, >> > > I do not know how do that. > > And I don't see the point, since from the "binary" POV, they > are "public". > > otherwise >> users may be tempted to directly use the classes there. >> > > You must have seen that there are prominent warnings against > doing that. > > Users will do what they want anyways. > What is not supported is not supported. We agreed here that it > was enough to tag a class as "internal" in order to fulfill our > obligation of least surprise. > > This would imply >> copying the documentation of the generators into the RandomSource class. >> > > I do not agree. > > Users can select a generator without knowing the gory details. > If they want to know more, they go to the internals. > It they really want to know, they go to the references. > > Internal should not be hidden: contributors who'd fancy to provide > additional generators do not have to go through any hoops. > > Or do you suggest that this component should supply two JARs: > * commons-rng-api > Contents of "o.a.c.rng" > * commons-rng-core > Contents of "o.a.c.rng.internal" > ? > > And two sets of Javadoc? > > - Why isn't the UniformRandomProvider interface simply named >> RandomProvider? >> > > Maybe because we want to be transparent on what the code does (?). > > With this name, there is no ambiguity: no more, no less than what > the name implies. > > Is there any plan to implement non uniform random >> generators in the future? Will they have a different interface? >> > > This point has been discussed on this list (although, again, there > was a severe lack of feedback...). > > Executive summary: > No strong coupling between generators (of uniform sequence of > pseudo-random numbers) and utilities on top of those generators > (e.g. generators producing non-uniform sequences). > Utilities should go in another component/package/module (see > e.g. Commons Math's packages "o.a.c.math4.distribution" and > "o.a.c.math4.random" for candidate code). > > - UniformRandomProvider mimics the java.util.Random methods which is >> nice. Is there any value in also implementing the nextGaussian() method >> such that our API is a perfect drop-in replacement for
Re: [RNG] About RNG-6 (Was: [VOTE][RC1] Release Commons Rng 1.0)
On Thu, Sep 15, 2016 at 7:37 AM, Gilles wrote: > Hi. > > Moving to another thread, as your comments do not belong in > a release vote. > > Many of them could lead to interesting discussions that should > (and could) have happened earlier. > > Indeed, for the record, see > https://issues.apache.org/jira/browse/RNG-6 > and the numerous posts which I sent to this list > * while ironing out the code of this new component in its own >repository, and doing the various chores (site, userguide, >quality testing, Github, Travis, Coveralls, etc) for the last >6 weeks, > * while developing that code in a new Commons Math package >"o.a.c.math4.rng" for the last 6 months, > * while fixing the code in Commons Math "o.a.c.m.random" >package for the last 9 months. > > For a number of issues which you now raise, I've explicitly > asked for input. > > I find it a bit strange that, at this precise point, you start > to question the choices which I've made in the absence of any > prior remarks, suggestions, or warnings. > > On Wed, 14 Sep 2016 17:04:53 +0200, Emmanuel Bourg wrote: > >> Hi all, >> >> RNG is moving fast, that's nice. I got a quick look at the API and the >> site, here are my comments: >> >> - The main page (Overview) could be enhanced with a description of the >> API longer than a single sentence. It could mention the motivations for >> the API, the implementations supported, a sample code snippet and a link >> to the user guide. >> > > I don't understand "motivations for the API". > > Do you mean something like my comment in > https://issues.apache.org/jira/browse/RNG-6 > ? > > [My motivation for proposing this code can be gathered from > the archives and the bugs and shortcomings reported in the > JIRA MATH project, and related posts here). They do not > belong in that "overview" page.] > > - In the Performance section of the User Guide, I'd suggest grouping the >> 3 tables into a single one. It might also be interesting to put the >> quality results in the same table, so we can see the trade-off between >> performance and quality. >> > > I won't do that. > It was already a pain to create those tables as they are (see the > doc source). > > - The License section of the user guide could be removed, I guess >> everyone knows that Apache projects use the Apache license. >> > > No problem. > I thought that it was mandatory. > > All the pages were copied from the corresponding Commons Math ones. > > - The internal packages should be hidden from the Javadoc, >> > > I do not know how do that. > Please see: https://maven.apache.org/plugins/maven-javadoc-plugin/javadoc-mojo.html#excludePackageNames Gary > > And I don't see the point, since from the "binary" POV, they > are "public". > > otherwise >> users may be tempted to directly use the classes there. >> > > You must have seen that there are prominent warnings against > doing that. > > Users will do what they want anyways. > What is not supported is not supported. We agreed here that it > was enough to tag a class as "internal" in order to fulfill our > obligation of least surprise. > > This would imply >> copying the documentation of the generators into the RandomSource class. >> > > I do not agree. > > Users can select a generator without knowing the gory details. > If they want to know more, they go to the internals. > It they really want to know, they go to the references. > > Internal should not be hidden: contributors who'd fancy to provide > additional generators do not have to go through any hoops. > > Or do you suggest that this component should supply two JARs: > * commons-rng-api > Contents of "o.a.c.rng" > * commons-rng-core > Contents of "o.a.c.rng.internal" > ? > > And two sets of Javadoc? > > - Why isn't the UniformRandomProvider interface simply named >> RandomProvider? >> > > Maybe because we want to be transparent on what the code does (?). > > With this name, there is no ambiguity: no more, no less than what > the name implies. > > Is there any plan to implement non uniform random >> generators in the future? Will they have a different interface? >> > > This point has been discussed on this list (although, again, there > was a severe lack of feedback...). > > Executive summary: > No strong coupling between generators (of uniform sequence of > pseudo-random numbers) and utilities on top of those generators > (e.g. generators producing non-uniform sequences). > Utilities should go in another component/package/module (see > e.g. Commons Math's packages "o.a.c.math4.distribution" and > "o.a.c.math4.random" for candidate code). > > - UniformRandomProvider mimics the java.util.Random methods which is >> nice. Is there any value in also implementing the nextGaussian() method >> such that our API is a perfect drop-in replacement for the JDK class? >> > > No. [IMHO.] > This was discussed on this list (and cf. above point). > > - For code relying on the java.util.Random type, it might be useful to
