Re: [core] Benjamin Kaduk's Discuss on draft-ietf-core-senml-more-units-04: (with DISCUSS and COMMENT)

[Benjamin Kaduk <[e-mail]>]

Hi Carsten,

Thanks for the updates; the changes in the -05 look good with just one
exception.  That, and some other comments, inline.

On Wed, Feb 19, 2020 at 01:50:57PM +0100, Carsten Bormann wrote:
> Hi Ben,
> 
> thank you for your in-depth review.
> 
> > ----------------------------------------------------------------------
> > DISCUSS:
> > ----------------------------------------------------------------------
> > 
> >   +-----------+-------------------+-------+-----------+-----+---------+
> >   | secondary | description       | SenML |     scale | off | refer-  |
> >   | unit      |                   | unit  |           | set | ence    |
> >   +-----------+-------------------+-------+-----------+-----+---------+
> >   [...]
> >   | km        | kilometer         | km    |      1000 |   0 | RFCthis |
> > 
> > The associated SenML unit would be just 'm', not 'km'.
> 
> Oops.  Obviously, it is a consistency requirement that anything in column 3 needs to be present in the primary unit name registration table.
> 
> Fixed in https://github.com/core-wg/senml-more-units/commit/78c714ab6a6526291265ec1976807ab3e8aa3138
> 
> 
> > ----------------------------------------------------------------------
> > COMMENT:
> > ----------------------------------------------------------------------
> > 
> > I appreciate the compromise position that was achieved after the long
> > last-call discussions questioning the need for this work.  It seems like
> > a reasonably shaped "escape hatch" to me.
> > 
> > Section 2
> > 
> > I assume this is just a product of my personal background, but I am more
> > used to "mass density" than "mass concentration" for dimensionalities
> > such as kg/m3.
> 
> True, the unit can be used for both quantities.
> (The actual request leading up to this was for particle concentration meters, hence the mass concentration label.)
> That factoid is actually covered in
> https://www.iso.org/obp/ui/#iso:std:iso:80000:-1:ed-1:v1:en
> Section 3.7, example 2.
> Since the registry captures units, not quantities, I propose adding mass density to the parenthesis.
> 
> Now in https://github.com/core-wg/senml-more-units/commit/25072a9e5757ea0d4aff75d2415afb40a17a9e37
> 
> > Section 3
> > 
> >   o  The Byte.  [IEC-80000-13] defines both the bit (item 13-9.b) and
> >      the byte (item 13-9.c, also called octet) as alternative names for
> >      the coherent unit one for the purpose of giving storage capacity
> >      and related quantities.  While the name octet is associated with
> > 
> > nit: is "one" misplaced, here?
> 
> The coherent unit actually is “one”; see Section 3.8, Note 3, and Section 3.12, Note 4 in ISO 80000-1 as cited above.
> To make this easier to read, we could place it in quotes, except it is not a string, it is really the number 1.

I guess I kind of gave it away that I didn't follow the reference, here :)
It's probably too verbose to try something like "the coherent unit used for
dimensionless quantities and indicated by the numeral one".

> > Section 4
> > 
> >   o  scale, offset: two rational numbers, expressed in decimal
> >      (optionally, with a decimal exponent given) or as a fraction
> >      divided by a "/" character.
> > 
> > nit: doesn't "a fraction divided by a '/' character" involve two '/'
> > characters?  That is, "1/2" is a fraction, so "1/2 divided by a '/'
> > character" would be like "1/2/" or "1/2/2" or something nonsensical like
> > that.
> 
> “divided” as in “separated into two halves”, i.e., “1/2” is divided by a “/“ into a 1 and a 2.  Maybe we need better phrasing here.

Perhaps, "a fraction represented using a '/' character to separtae
numerator and denominator"?

> > It's a little surprising to see kibibyte and gigabyte defined but not
> > gibibyte.
> 
> It’s a registry, nobody asked for it yet…
> We could do some more proactive registering.
> 
> >   names.  Guidelines to the difference between units (which can go into
> >   the registry) and quantities are widely available, see for instance
> >   [RS] and [BIPM].
> > 
> > I suggest a parenthetical "(which cannot)" after "quantities".
> 
> Very good.
> 
> Now in https://github.com/core-wg/senml-more-units/commit/25072a9e5757ea0d4aff75d2415afb40a17a9e37
> 
> > 
> >   Where the benefits of directly using a secondary unit in a SenML pack
> >   outweigh the obove considerations, the use of secondary units in "u"
> > 
> > nit: s/obove/above/
> 
> (Covered earlier.)
> 
> >   specifically allows this and/or by using a field with a label name
> >   that ends with the "_" character ("must-understand" field) that
> >   specifically allows this.  The definition of these versions and
> > 
> > I suggest "whose definition specifically allows this".
> 
> Good point.
> 
> Now in
> https://github.com/core-wg/senml-more-units/commit/76f51849541b5a18a6ef2984507284d92514eb54
> 
> > 
> >   fields is outside the scope of the present specification; one such
> >   definition is provided in [I-D.bormann-core-senml-versions].
> > 
> > nit: I suggest "proposed definition" or "possible definition", as
> > internet-drafts are "works in progress".
> 
> Maybe shorter “..is proposed in”?
> I’ve put that into
> https://github.com/core-wg/senml-more-units/commit/8f2163a311e088a8cdfc30fa37df3a9f175c87e3

Sure.

> > Section 5
> > 
> >   potential single point of failure.  Instead of pulling the registry
> >   in each individual instance of the code, the software update
> >   mechanism (or a similar, less frequently triggered mechanism) SHOULD
> >   be used to disseminate updated units registries obtained from IANA
> >   towards the instances via common repositories.
> > 
> > I like how this blandly assumes that a software update mechanism is
> > available.  Optimistic for the present-day world, but still probably the
> > right thing to say.
> > nit: as written, "less frequently triggered mechanism" means "less
> > frequently triggered than the software update mechanism", which I
> > suspect is not the intent.
> 
> I have turned this into
> 
> (or a similar
> mechanism visiting IANA less frequently)

This is better, but I think that "less frequently" is still an implied
comparison, and the comparison against software-update is more local than
the (intended) comparison against the generic/abstract "usual frequency" or
"every time an instance of the system starts up or checks".  I might
suggest "a similar mechanims that only visits IANA infrequently".

> in https://github.com/core-wg/senml-more-units/commit/8656620080aa6d0d2a422476a7a5693e931e6f3e
> 
> > Section 6
> > 
> > I suggest reiterating the implications of the requirements at the end of
> > section 4, namely, that use of new units is "fail-safe", in that an
> > implementation processing a pack using secondary units is guaranteed to
> > have been developed with an awareness of the risks of having multiple
> > units available for the same logical type.  (It is not, of course,
> > guaranteed to know about the specific secondary unit in question when
> > just the "SenML Version" check is used, in the same way that current
> > SenML applications are not guaranteed to know about units not in the
> > initial allocation from RFC 8428.)
> > 
> >   The security considerations of [RFC8428] apply.  The introduction of
> >   new measurement units poses no additional security considerations
> >   except from a possible potential for additional confusion about the
> >   proper unit to use.
> > 
> > Nitpicking, but I think there's a related risk of confusion in terms of
> > what quantity has been measured and is indicated in a SenML pack, as
> > alluded to by the previous mention of "trigger[ing] on the presence of a
> > quantity in a certain unit".
> 
> I have tried to cover this in
> https://github.com/core-wg/senml-more-units/commit/532dce8315bf9c75af1d1150d078d2f91e6444c0

Thanks for all the updates; I've removed my Discuss accordingly.

-Ben