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

Carsten Bormann <cabo@tzi.org> Wed, 19 February 2020 12:51 UTC

Return-Path: <cabo@tzi.org>
X-Original-To: core@ietfa.amsl.com
Delivered-To: core@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 9802A12003F; Wed, 19 Feb 2020 04:51:03 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.2
X-Spam-Level:
X-Spam-Status: No, score=-4.2 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id UthAIQwI9y35; Wed, 19 Feb 2020 04:51:00 -0800 (PST)
Received: from gabriel-vm-2.zfn.uni-bremen.de (gabriel-vm-2.zfn.uni-bremen.de [134.102.50.17]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 26828120019; Wed, 19 Feb 2020 04:51:00 -0800 (PST)
Received: from client-0171.vpn.uni-bremen.de (client-0171.vpn.uni-bremen.de [134.102.107.171]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by gabriel-vm-2.zfn.uni-bremen.de (Postfix) with ESMTPSA id 48MyJL1pcmzyV6; Wed, 19 Feb 2020 13:50:58 +0100 (CET)
Content-Type: text/plain; charset="utf-8"
Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3608.60.0.2.5\))
From: Carsten Bormann <cabo@tzi.org>
In-Reply-To: <158207175238.13976.4748538225663851323.idtracker@ietfa.amsl.com>
Date: Wed, 19 Feb 2020 13:50:57 +0100
Cc: The IESG <iesg@ietf.org>, draft-ietf-core-senml-more-units@ietf.org, Jaime Jimenez <jaime@iki.fi>, core-chairs@ietf.org, core@ietf.org
X-Mao-Original-Outgoing-Id: 603809457.787553-71f13adf809e79555dbeba34af29dde0
Content-Transfer-Encoding: quoted-printable
Message-Id: <89E1D428-FF1B-4B42-BF1A-2BEF9B2C0B1B@tzi.org>
References: <158207175238.13976.4748538225663851323.idtracker@ietfa.amsl.com>
To: Benjamin Kaduk <kaduk@mit.edu>
X-Mailer: Apple Mail (2.3608.60.0.2.5)
Archived-At: <https://mailarchive.ietf.org/arch/msg/core/VIjRg84VrJFy4pRnauCT_OClX3E>
Subject: Re: [core] Benjamin Kaduk's Discuss on draft-ietf-core-senml-more-units-04: (with DISCUSS and COMMENT)
X-BeenThere: core@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "Constrained RESTful Environments \(CoRE\) Working Group list" <core.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/core>, <mailto:core-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/core/>
List-Post: <mailto:core@ietf.org>
List-Help: <mailto:core-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/core>, <mailto:core-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 19 Feb 2020 12:51:04 -0000

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.

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

> 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

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

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

Grüße, Carsten