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

Benjamin Kaduk <kaduk@mit.edu> Wed, 19 February 2020 17:01 UTC

Return-Path: <kaduk@mit.edu>
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 C589F120914; Wed, 19 Feb 2020 09:01:37 -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 LsIx-YoBxaDG; Wed, 19 Feb 2020 09:01:32 -0800 (PST)
Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 22EC41208CE; Wed, 19 Feb 2020 09:01:31 -0800 (PST)
Received: from kduck.mit.edu ([24.16.140.251]) (authenticated bits=56) (User authenticated as kaduk@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id 01JH1RgZ008251 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 19 Feb 2020 12:01:29 -0500
Date: Wed, 19 Feb 2020 09:01:26 -0800
From: Benjamin Kaduk <kaduk@mit.edu>
To: Carsten Bormann <cabo@tzi.org>
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
Message-ID: <20200219170126.GE11645@kduck.mit.edu>
References: <158207175238.13976.4748538225663851323.idtracker@ietfa.amsl.com> <89E1D428-FF1B-4B42-BF1A-2BEF9B2C0B1B@tzi.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <89E1D428-FF1B-4B42-BF1A-2BEF9B2C0B1B@tzi.org>
User-Agent: Mutt/1.12.1 (2019-06-15)
Archived-At: <https://mailarchive.ietf.org/arch/msg/core/ufSaSza2nurvdEjhW3zL9EGHfQQ>
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 17:01:44 -0000

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