Subject: [tsvwg] Roman Danyliw's Discuss on draft-ietf-tsvwg-tinymt32-03: (with DISCUSS and COMMENT)

From: Roman Danyliw via Datatracker <[email-masked]>
Date: Tue, 28 May 2019 17:52:40 -0700

Roman Danyliw has entered the following ballot position for
draft-ietf-tsvwg-tinymt32-03: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-tsvwg-tinymt32/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

(1) Section 1.  Per “The TinyMT32 PRNG initialization depends, among other
things, on a parameter set -- namely (mat1, mat2, tmat) -- that needs to be
well chosen (pre-calculated values are available in the official web site).”,
why is there a reference to an external website when Section 3.1 explicitly
lists the constant and mandates their usage?  I’m elevating this to a discuss
as there should be no ambiguity on the location of these constants.

(2) Section 3.1.  A comment in tinymt32_next_state() notes code changes made to
the “original code … [so it is] not depend on the representation of negative
integers by 2's complements.”  The following code in tinymt32_temper() appears
to suffer from this same, original portability issue: t0 ^= -((int32_t)(t1 &
1)) & s->tmat;

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

(1) Section 1.  “BigCrush in TestU01 and AdaptiveCrush” needs a reference.

(2) Abstract and Section 1.  I found characterizing the quality of the
randomness as “pretty good” imprecise (relative to what?).  Can this statement
be qualified or qualified in some way?

(3) Section 1 (“This is the first difference …”) and Section 3.1 (“The TinyMT32
PRNG reference implementation is reproduced in Figure 1, with the following
differences …”) both enumerate a list of differences between the github repo
and the code in this text.  Why are both required? Section 3.1 seems more
precise.

(4) Section 1.  I am confused by the terminology of “official web site” and
“official github site”.  What makes them official?

(5) Section 1.  With due respect to the first two authors, it seems odd to
mention them by name three times in this section.  Also, the fact that this
document is derived from a github repo is mentioned twice in this section and
once in the code comments of Section 3.1.

(6) Section 3.1.  There are normative C code blocks in this text.  The obvious
fact that this is C code needs to be specified – consider C99 (ISO/IEC
9899:1999), C11 (ISO/IEC 9899:2011), C18 (ISO/IEC 9899:2018)

(7) Section 3.1.  I agree with ekr’s original comment that hard coded a
constant in tinymt32_init() makes the code less readable -- “s->status[i & 3]
^= i + UINT32_C(1812433253)”

(8) Editorial Nits

Abstract.  No references are permitted in the abstract.
Section 1.  Typo.  s/variatnt/variant/
Section 1.  Typo.  s/randomnes/randomness/
Section 3.1.  Typo.  s/licence/license/