Re: [VOTE][RC2] Release Commons Math 3.6

[Luc Maisonobe]

Le 02/01/2016 21:15, Luc Maisonobe a écrit :
> This is a [VOTE] for releasing Apache Commons Math 3.6 from release
> candidate 2.
> 
> Tag name:
>   MATH_3_6_RC2 (signature can be checked from git using 'git tag -v')
> 
> Tag URL:
> 
> 
> 
> Commit ID the tag points at:
>   95a9d35e77f70ffc9bd5143880c236a760b42005
> 
> Site:
>   
> 
> Distribution files:
>   https://dist.apache.org/repos/dist/dev/commons/math/
> 
> Distribution files hashes (SHA1):
>  8b0b724b91102f63c7211f8de60dec19f94c4af7  commons-math3-3.6-bin.tar.gz
>  f3f2b3974e5ecd368aa3294f8c59a7aeae4c5d90  commons-math3-3.6-bin.zip
>  213985b076b344178cd2fb07e7257cb52b65e3b9  commons-math3-3.6-src.tar.gz
>  0c66a1aaf878aa7e6c0d9bbf917b25efa463b313  commons-math3-3.6-src.zip
> 
> KEYS file to check signatures:
>   http://www.apache.org/dist/commons/KEYS
> 
> Maven artifacts:
> 
> https://repository.apache.org/content/repositories/orgapachecommons-1138/org/apache/commons/commons-math3/3.6/
> 
> [X] +1 Release it.

Luc

> [ ] +0 Go ahead; I don't care.
> [ ] -0 There are a few minor glitches: ...
> [ ] -1 No, do not release it because ...
> 
> This vote will close in 72 hours, at 2016-01-05T20:15:00Z (this is UTC
> time).
> 
> 
> 
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
> 
> 


-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[All] Don't touch that code! (Was: [Math] About the refactoring of RNGs)

[Gilles]

Hi.

[Preamble: In the following, I do not ask for opinions about the
design of Commons Math; I report an example of a dysfunctional
"community".]

A few days ago, rather than "shut down for holidays", I took upon me to
respond to the request of a newcomer to the CM discussions (MATH-1300).
I'm certainly not anywhere close to being an expert in PRNG.
But it looked easy enough to make that change.

While browsing through the related code in "o.a.c.m.random", I happened 
to
stumble upon one line of code that looked strange (because its sole 
purpose

seemed to clear bits that, anyways, would not be used by the caller).

Then there were the usual suspects when doing such an exercise in CM:
duplicate code ("BitsStreamRandomGenerator" vs 
"AbstractRandomGenerator"),

and unsafe code, that prompted me to raise questions on this ML, and to
open issues MATH-1307, MATH-1308 and MATH-1309 (itself related to the 
old

MATH-734).

My argumentation was crystal-clear.
No technical objection was raised, and I committed the proposed changes 
(as

they were shown in an attachment to the corresponding JIRA page).

The commit elicited a flood of negative comments that made me feel I 
must

have made some horrendous, and obviously stupid, mistake.

Around the same time I was making those little changes (creating 1 new 
file,

deleting 2, and less than 20 lines changed in total) to the development
branch ("master") containing code not supposed to be 
backwards-compatible
and not foreseen to be released any time soon, Luc was adding or 
modifying
dozens of files (largely more than 1000 lines to supposedly review) to 
the

MATH_3_X branch, thus to be included in the 3.6 version of CM to be
imminently released.

There is thus a double-standard reviewing process.[1]

As it turned out, nobody has, at this time, something technically sound 
to

argue against the change which I had made.

Instead, there was a barrage of protests that amounted to creating FUD:

FUD 1: "Quite possibly, yes, you are missing something."
Truth: Strong, but baseless, affirmation that I'm wrong in even 
thinking

   that some code statement could be useless.

FUD 2: "significant refactoring"
Truth: Less than 20 lines were modified (and for many, it's the same 
type

   of change).
   [Moreover, the behaviour and usage of the concrete classes are
   unchanged.]

FUD 3: "[...] rebasing to eliminate next(bits) adds nothing."
Truth: New code exposes what the underlying algorithms actually do 
while
   the previous code indirectly advertised non-existing 
functionality.

   [The change actually proves that the argument of "next(int)" is
   useless (i.e. the implementations always generate more bits than 
the

   argument would have one to believe).]

FUD 4: "[refactoring] to eliminate the (standard) next(int)"
Truth: Unchecked and ambiguous use of the word "standard".
   As far I could see by browsing some source codes in C, 
"next(void)"

   could be the standard in RNG codes, not "next(int bits)".
   The absence of "next(int)" in the newer JDK class 
"SplittableRandom"
   confirms the impression that "next(int)" could have been, 
indeed,

   useless for all practical purposes.

FUD 5: Potential breakage
Truth: 544 unit tests show no sign of regression for any of the RNG
   implementations.

FUD 6: "The tests cover only nextInt"
Truth: The "RandomGeneratorAbstractTest" class contains unit tests for 
*all*

   the generic methods.  In particular, the tests ensure that the
   distribution of the generated "int", "long", "float" and 
"double" is

   uniform.

FUD 7: "[...] implementation changes to hardened code"
Truth: Undefined use of the word "hardened": A specific unit test 
verifies
   that some hard-coded sequences are reproduced by the code. It 
passes

   with the new code, as it did with the old.
   [Other than that, the code comes with no warranty (applies to 
*both*

   the current version and the previous one).]

FUD 8: "[...] invariants may be broken by these changes"
Truth: Visual inspection of 1 line of code per generic method and 1 
line of

   code per RNG can prove whether there is a mistake or not.

FUD 9: "[...] possibility of introducing [...] performance issues."
Truth: Visual inspection of 1 line of code per generic method and 1 
line of
   code per RNG shows a reduction of the number of basic operations 
(one

   shift and one subtraction in each concrete RNG implementation).
   Moreover, IIUC the following results of a micro-benchmark 
comparison,

   no significant change in performance can be detected.
   [Although the change is for the better in almost all cases.]

+--+
| MATH_3_X |
+--+
nextInt() (calls per timed block: 200, timed blocks: 100, time 
unit: ms)
  name  time/call  std error total time  ratio  
difference
JDKRandomGenerator 1.07711988e-05 3.96937236e-06 2.1542e+03 1.e+