On Thu, 9 Aug 2018 09:25:13 -0400, Rob Tompkins wrote:
On Aug 9, 2018, at 9:17 AM, Gilles <gil...@harfang.homelinux.org>
wrote:
On Thu, 9 Aug 2018 08:43:34 -0400, Rob Tompkins wrote:
On Aug 9, 2018, at 8:38 AM, Gilles <gil...@harfang.homelinux.org>
wrote:
On Thu, 9 Aug 2018 08:22:32 -0400, Rob Tompkins wrote:
Are we instead trying to remove the “extends” from all of those
classes?
I'm not sure what you mean, sorry.
"SamplerBase" was meant has a utility/shortcut/boiler-plate in
order
to copy the "rng" argument in one place, instead of having a field
in each sampler class. It is purely internal to this library (in
C++,
inheritance would have been "private”).
Are we trying to remove SamplerBase all together?
That would break BC the same way.
I wouldn't mind since the shortcut is not really significant.
But my reasoning would be the same: correct usage does not refer
to these methods so the fix could be done now, with minimal risk
(due to low usage of a new component).
My thought was to override the method and javadoc locally (in the
actual subclass) so that we can give reasonable deprecation
messages,
but that’s only a thought. I’m ok with whatever.
It's overkill in this context.
We shouldn't be rigid about the "no BC in minor release" rule;
I’m a tad confused here. I thought that the whole point of everything
that we do in terms of the way we version and name packages
was specifically driven at the fact that we’re completely rigid about
“no BC breakage in minor releases.”
I’ve not been around the project long enough to know this with
certainty, but that definitely is the vibe that I get. I would think
that such RCs would generally get -1’s from people.
Right. Except when the rationale for breaking BC shows that no bona
fide codes can suffer from the incompatibility. This had already
been discussed, and accepted, for this very release a few months
ago (cf. "BoxMullerLogNormalSampler"[1]).
Generally I’m open to whatever. I’m just operating on what I perceive
to be precedent.
I understand but there is a slight difference between being nice
to people and getting clean reports; don't you think? ;-)
Gilles
[1] https://issues.apache.org/jira/browse/RNG-46
@Gary - what’s the call here on the flexibility of BC incompatible
changes in a minor release?
enforcing it is worse for everybody:
* bona fide users will be forced to modify their source and
recompile in order to benefit from the performance boost
introduced in version 1.1, and
* bad usage (if it exists at all) could continue unsuspected.
The message would aim at the wrong target: for developers of this
library, there is no deprecation (the boiler-plate code is there
to be used); for application developers, the class should not be
there to be used (hence: package private).
Then, I guess, we should simply state that we want to eventually
delete these methods and they should not be consumed. Yeah?
No (cf. above); I propose to delete them *now*.
Like I
said…I’m up for whatever. I’m just trying to balance yours and
Gary’s
points and find a good middle ground that everyone can live with.
Which are the two sides for which you try to find a middle ground?
At any rate it would not be "good" if it makes developers and users
unhappy for a case that probably doesn't exist out there.
Tools such as Clirr are a great help, but they shouldn't induce us
to take the wrong course only to have the pleasure to contemplate a
clean report. ;-)
Thanks to Clirr (and Gary), we've become aware of a design issue.
We can decide to resolve it as it was done in the past, with IMHO
zero inconvenience (except for the Clirr "unclean" report).
Regards,
Gilles
-Rob
Gilles
-Rob
On Aug 9, 2018, at 7:02 AM, Gilles
<gil...@harfang.homelinux.org> wrote:
Hi all.
On Wed, 08 Aug 2018 19:59:24 +0200, Gilles wrote:
Hello Rob.
On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote:
@Gilles - thoughts here?? Just kinda what I was thinking, but
I’m
only a +0 on this change. So, if you want to revert it before
going up
with 1.1, that’s fine.
I don't understand this change after what I answered to Gary's
strange
proposal.
The comment is *wrong*. As I said previously, the base class
provides
boiler-plate code; that is *not* deprecated. The issue is that
those
methods were not meant to be used further down the hierarchy
(in user's
subclasses). [And now, furthermore, they are not used anymore
in class
"PoissonSampler", following the change in RNG-50 (delegation to
other
classes).]
The sampler classes should have been made "final"; but now,
this change
would also be "breaking" (even though I doubt about legitimate
use-cases
for inherithing from the sampler implementations).
Assuming it's unlikely that application developers would have
* called protected methods of "SamplerBase",[1]
* create subclasses of "SamplerBase",[1]
* create subclasses of the sampler classes,[1]
I suggest to
* make "SamplerBase" package private
* make the sampler clases "final".
The above are the *less* intrusive fixes, as they would only
potentially impact application developers by making them aware
that they have made incorrect usage of an *inadvertently*
public API. Correct usage will not be impacted even though
the change is not BC.[2]
Any objection to that fix?[3]
Regards,
Gilles
[1] Rationale: (a) There is no reason to do that and
(b) "Commons RNG" isn't much used at this point:
http://mvnrepository.com/artifact/org.apache.commons/commons-rng-sampling/usages
[2] I recall that a similar situation (BC breaking in a minor
release) occurred in CM, at a time where the number of
potential users was much larger.
[3] I'll add a prominent warning in the changelog to the effect
that people wanting to continue with incorrect usage of the
samplers should not upgrade. ;-)
Please revert.
Thanks,
Gilles
-Rob
On Aug 8, 2018, at 12:42 PM, chtom...@apache.org wrote:
Repository: commons-rng
Updated Branches:
refs/heads/1.1 50b984b1d -> f8159f28a
Adding PoissonSampler deprecations. Use the correct faster
public methods
Project:
http://git-wip-us.apache.org/repos/asf/commons-rng/repo
Commit:
http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
Tree:
http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
Diff:
http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
Branch: refs/heads/1.1
Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
Parents: 50b984b
Author: Rob Tompkins <chtom...@gmail.com>
Authored: Wed Aug 8 12:42:45 2018 -0400
Committer: Rob Tompkins <chtom...@gmail.com>
Committed: Wed Aug 8 12:42:45 2018 -0400
----------------------------------------------------------------------
.../sampling/distribution/PoissonSampler.java | 42
++++++++++++++++++++
1 file changed, 42 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
----------------------------------------------------------------------
diff --git
a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
index d0733ba..29d0e4e 100644
---
a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
+++
b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
@@ -67,6 +67,48 @@ public class PoissonSampler
return poissonSampler.sample();
}
+ /**
+ * @return a random value from a uniform distribution in
the
+ * interval {@code [0, 1)}.
+ * @deprecated - one should be using the {@link
PoissonSampler#sample()} method,
+ * as it is considerably faster.
+ */
+ @Deprecated
+ protected double nextDouble() {
+ return super.nextDouble();
+ }
+
+ /**
+ * @return a random {@code int} value.
+ * @deprecated - one should be using the {@link
PoissonSampler#sample()} method,
+ * as it is considerably faster.
+ */
+ @Deprecated
+ protected int nextInt() {
+ return super.nextInt();
+ }
+
+ /**
+ * @param max Upper bound (excluded).
+ * @return a random {@code int} value in the interval
{@code [0, max)}.
+ * @deprecated - one should be using the {@link
PoissonSampler#sample()} method,
+ * * as it is considerably faster.
+ */
+ @Deprecated
+ protected int nextInt(int max) {
+ return super.nextInt(max);
+ }
+
+ /**
+ * @return a random {@code long} value.
+ * @deprecated - one should be using the {@link
PoissonSampler#sample()} method,
+ * * as it is considerably faster.
+ */
+ @Deprecated
+ protected long nextLong() {
+ return super.nextLong();
+ }
+
/** {@inheritDoc} */
@Override
public String toString() {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
