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;
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
