Re: [netmod] AD review of draft-ietf-netmod-geo-location-07

["Rob Wilton (rwilton)" <[e-mail]>]

Hi Chris,

> -----Original Message-----
> From: Christian Hopps <[e-mail]>
> Sent: 13 April 2021 03:38
> To: Rob Wilton (rwilton) <[e-mail]>
> Cc: Christian Hopps <[e-mail]>; [e-mail]; draft-ietf-
> [e-mail]
> Subject: Re: AD review of draft-ietf-netmod-geo-location-07
> 
> 
> 
> > On Mar 7, 2021, at 3:48 PM, Rob Wilton (rwilton) <[e-mail]>
> wrote:
> >
> > Hi Chris,
> >
> > Apologies for the delayed AD review of this document.
> >
> > I found that this document to be interesting, enlightening, and well
> written, so thank you.
> >
> >
> > I only have a few minor comments, the rest are grammatical nits. Some I
> spotted manually; the rest are from a beta version of a grammar tool that
> I am playing with.
> >
> > Minor comments/questions:
> >
> > 1.
> > In the YANG module, the pattern statements have 3 ranges of characters,
> > but the description only indicates two ranges.  Is there a reason that
> > the following pattern doesn't work?
> >  pattern '[ -@\[-~]*';
> 
> From an old email on the list: "These 3 ranges seem to make pyang happy. I
> don't know why I need to break up the second range into 2 adjacent ranges
> like that to make pyang not complain, but complain it does if I just use:
> '[ -@\[-~]*'"
[RW] 

Okay. 


> 
> > 2.
> > I note that the YANG module allows just a lat, or a long, or a height
> > to be specified, rather than requiring at least a pair of lat/long or
> > x/y coordinates.  I think that this is fine (and keeps the model
> > flexible), but wanted to check that this is the intentional.
> 
> > 3.
> > I also note that that grouping doesn't require that any coordinates be
> > specified at all.  I presume that this is intentional, it makes sense
> > to me (e.g., if it is intended to be optional).
> 
> My answer to a thread on the list asking for removal of some of the
> mandatory designations which cause problems:
> 
> "The intention was for a location node to be required if the <geo-
> location> container was present. Apparently this will only work if
> "container geo-lcoation" is a presence container. I'm not even sure that's
> all that smart of a requirement (e.g., maybe someone just wants to
> indicate the reference-frame for an object). I'll remove the mandatory
> from location."
[RW] 

Okay.


> 
> 
> > 4. In the YANG module,
> > "v-east is the rate of change (i.e., speed) perpendicular
> > to truth-north as defined by the geodetic-system.";
> >
> > As a nit, this doesn't actually define whether a positive v-east
> > value is in the East or West direction.  I appreciate that this is
> > obvious, but for the other two components in the vector, it is
> > unambiguously specified.
> 
> How about:
> 
>               "v-east is the rate of change (i.e., speed) perpendicular
>                to the right of truth-north as defined by
>                the geodetic-system.";
[RW] 

LGTM.


> 
> 
> >
> > 5.
> > In Security Considerations:
> >
> >   Since the grouping defined in this module identifies locations,
> >   authors using this grouping SHOULD consider any privacy issues that
> >   may arise when the data is readable.
> >
> > Perhaps, expand this paragraph to give an example, e.g., revealing
> > the physical location of a device, or data center.
> 
> How about:
> 
> "Since the grouping defined in this module identifies locations,
> authors using this grouping SHOULD consider any privacy issues
> that may arise when the data is readable (e.g., customer device
> locations, etc)."
[RW] 

LGTM.


> 
> 
> >
> > The rest are just grammar nits:
> >
> >
> >   In addition to identifying the astronomical body we also need to
> >   define the meaning of the coordinates
> > =>
> >   In addition to identifying the astronomical body, we also need to
> >   define the meaning of the coordinates
> 
> fixed
> 
> >
> >   In addition to the "geodetic-datum" value we allow refining the
> >   coordinate and height accuracy using "coord-accuracy" and "height-
> >   accuracy" respectively.
> > =>
> >   In addition to the "geodetic-datum" value, we allow refinement of the
> >   coordinate and height accuracy using "coord-accuracy" and "height-
> >   accuracy" respectively.
> 
> fixed
> 
> >   This is the location on or relative to the astronomical object.  It
> >   is specified using 2 or 3 coordinates values.
> > =>
> >   This is the location on, or relative to, the astronomical object.  It
> >   is specified using 2 or 3 coordinates values.
> 
> fixed
> 
> >   The intent of the grouping being defined here is to identify
> >   where something is located, and generally this is expected to be
> >   somewhere on or relative to Earth (or another astronomical body).
> > =>
> >   The intent of the grouping being defined here is to identify
> >   where something is located, and generally this is expected to be
> >   somewhere on, or relative to, Earth (or another astronomical body).
> 
> fixed
> 
> >   At
> >   least two options are available to YANG models that wish to use this
> >   grouping with objects that are changing location frequently in non-
> >   simple ways, they can add additional motion data to their model
> >   directly, or if the application allows it can require more frequent
> >   queries to keep the location data current.
> > =>
> >   At
> >   least two options are available to YANG models that wish to use this
> >   grouping with objects that are changing location frequently in non-
> >   simple ways.  They can add additional motion data to their model
> >   directly.  Or, if the application allows, it can require more frequent
> >   queries to keep the location data current.
> 
> fixed
> 
> > When coord-accuracy is specified it overrides the geodetic-datum implied
> > accuracy.
> > =>
> > When coord-accuracy is specified, it overrides the geodetic-datum
> implied
> > accuracy.
> 
> fixed
> 
> >
> > When specified it overrides the geodetic-datum implied default.
> > =>
> > When specified, it overrides the geodetic-datum implied default.
> 
> fixed
> 
> > indicated by the reference-frame value.
> > =>
> > indicated by the reference-frame.
> 
> fixed
> 
> > For a formula to convert these values to speed and heading see
> > this modules defining document RFC XXXX.";
> > =>
> > For a formula to convert these values to speed and heading see
> > RFC XXXX.";
> 
> fixed
> 
> > You have "truth-north" and "truth north" and "true-north".  Should
> > these all be "true north"?
> 
> fixed
> 
> >
> >   YANG grouping using decimal64 values rather than strings.  For the
> >   relative height cases the application doing the transformation is
> >   expected to have the data available to transform the relative height
> >   into an absolute height which can then be expressed using the YANG
> >   grouping.
> > =>
> >   YANG grouping using decimal64 values rather than strings.  For the
> >   relative height cases, the application doing the transformation is
> >   expected to have the data available to transform the relative height
> >   into an absolute height, which can then be expressed using the YANG
> >   grouping.
> 
> fixed
> 
> > Grammar Warnings (generated by a tool):
> > Draft Text:
> > Indeed it is easy to imagine a network or device located on The Moon, on
> Mars, on Enceladus (the moon of Saturn) or even a comet (e.g.,
> 67p/churyumov-gerasimenko).
> >
> > Warning:  Did you forget a comma after a conjunctive/linking adverb?
> > Suggested change:  "Indeed,"
> 
> fixed
> 
> >
> > Draft Text:
> > This document defines a "geo-location" YANG grouping that allows for all
> of the above data to be captured.
> >
> > Warning:  Consider using all the.
> > Suggested change:  "all the"
> 
> fixed
> 
> > Draft Text:
> > When specified these values override the defaults implied by the
> "geodetic-datum" value.
> >
> > Warning:  "When" at the beginning of a sentence usually requires a 2nd
> clause. Maybe a comma, question or exclamation mark is missing, or the
> sentence is incomplete and should be joined with the following sentence.
> > Suggested change: "When specified, "
> 
> fixed
> 
> > Draft Text:
> > In both choices the exact meanings of all of the values are defined by
> the "geodetic-datum" value in the [xref].
> >
> > Warning:  Consider using all the.
> > Suggested change:  "all the"
> 
> fixed
> 
> > Draft Text:
> > During the development of this module, the question of whether it would
> support data such as orientation arose.
> > Warning:  Wordiness: Consider shortening this phrase.
> > Suggested change:  "whether"
> 
> ?
[RW] 

Ignore.


> 
> >
> > Draft Text:
> > For test "A.1.2.1" the YANG geo location object either includes a CRS
> ("reference-frame") or has a default defined ([xref]).
> >
> > Warning:  This word is normally spelled as one.
> > Suggested change:  "geolocation"
> 
> unchanged.
> 
> > Draft Text:
> > Many systems make use of geo-location data, and so it's important to be
> able describe this data using this geo-location object defined in this
> document.
> >
> > Warning:  The preposition 'to' is required in front of the verb
> 'describe'.
> > Suggested change:  "able to describe"
> 
> Removed this paragraph it's unneeded and not very clear.
> 
> >
> > Draft Text:
> > For accuracy it has a single "u" parameter for specifying uncertainty.
> > Warning:  The comma is probably missing here: accuracy, it.
> > Suggested change:  "accuracy, it"
> 
> fixed
> 
> > Draft Text:
> > This is used by many application (e.g., Google Maps API).
> >
> > Warning:  Possible agreement error. The noun application seems to be
> countable; consider using: many applications.
> > Suggested change:  "many applications"
> 
> fixed
> 
> > Draft Text:
> > Thus GML "gml:pos" values can be mapped directly to the YANG grouping,
> with the caveat that some loss of precision (in the extremes) may occur
> due to the YANG grouping using decimal64 values rather than doubles.
> >
> > Warning:  Did you forget a comma after a conjunctive/linking adverb?
> > Suggested change:  "Thus,"
> 
> fixed
> 
> > Draft Text:
> > Furthermore "gml:validTime" can either be an Instantaneous measure
> ("gml:TimeInstant") or a time period ("gml:TimePeriod").
> > Warning:  Did you forget a comma after a conjunctive/linking adverb?
> > Suggested change:  "Furthermore,"
> 
> fixed
> 
> > Draft Text:
> > As with the "kml:altitudeMode" value, the YANG grouping supports the
> ignore case but not the relative case.
> >
> > Warning:  After 'the', do not use a verb. Make sure that the spelling of
> 'ignore' is correct. If 'ignore' is the first word in a compound
> adjective, use a hyphen between the two words. Note: This error message
> can occur if you use a verb as a noun, and the word is not a noun in
> standard English.
> 
> I think it just wants me to hyphenate "ignore-case". "ignore" is an
> adjective here "case" is the noun. *shrug*, I think it reads fine the way
> it is for a technical document.
[RW] 

Yes, please just ignore.  The tool isn't perfect.



> 
> > Draft Text:
> > Thus the YANG grouping and KML values can be directly mapped in both
> directions (when using a supported altitude mode) with the caveat that
> some loss of precision (in the extremes) may occur due to the YANG
> grouping using decimal64 values rather than strings.
> > Warning:  Did you forget a comma after a conjunctive/linking adverb?
> > Suggested change:  "Thus,"
> 
> fixed
> 
> > Draft Text:
> > The allocation policy for this registry is First Come First Served,
> [xref] as the intent is simply to avoid duplicate values.
> >
> > Warning:  It seems that a comma is missing.
> > Suggested change:  "Come,"
> 
> fixed
> 
> > Draft Text:
> > All of the data nodes defined in this YANG module are
> writable/creatable/deletable (i.e., "config true", which is the default).
> > Warning:  Consider using all the.
> > Suggested change:  "All the"
> 
> fixed
> 
> > Draft Text:
> > These are the subtrees and data nodes and their
> sensitivity/vulnerability:
> > None of the writable/creatable/deletable data nodes in the YANG module
> defined in this document are by themselves considered more sensitive or
> vulnerable then standard configuration.
> >
> > Warning:  Did you mean than?
> > Suggested change:  "than"
> 
> fixed
> 
> > Draft Text:
> > Some of the readable data nodes in this YANG module may be considered
> sensitive or vulnerable in some network environments.
> > Warning:  If the text is a generality, 'of the' is not necessary.
> > Suggested change:  "Some"
> 
> I think it's correct to leave this, but I could be misunderstanding the
> advice.
[RW] 

I think that either is probably fine/correct.


> 
> > Draft Text:
> > Below is a the YANG tree for the fictitious module that uses the geo-
> location grouping.
> >
> > Warning:  Maybe you need to remove one determiner so that only a or the
> is left.
> > Suggested change:  "a"
> 
> fixed with "the" only.
> 
> >
> > Draft Text:
> > We would also like to thank Peter Lothberg for the motivation as well as
> help in defining a broadly useful geographic location object, and Acee
> Lindem and Qin Wu for their work on a geographic location object that led
> to this documents creation.
> >
> > Warning:  Possible typo: apostrophe is missing. Did you mean documents'
> or document's?
> > Suggested change:  "documents'"
> 
> fixed.
> 
> Nice tool!
[RW] 

Thanks.

It needs a bit more time and work, but the hope is that it can be an easy webtool that can be used by authors to help check the grammar/spelling in their drafts.  I don't want it to be an id-nits style gatekeeper tool, because as you have seen, some of the warnings/suggestions are somewhat arbitrary.  Also, the real credit goes to an open source grammar tool (dev.languagetool.org) that is doing all the heavy lifting.

Thanks,
Rob


> 
> Thanks,
> Chris.
> 
> 
> >
> > Regards,
> > Rob
> >