Closed ajnelson-nist closed 1 year ago
This issue has nothing to do specifically with Facets.
I will save comments on the myriad of issues, incomplete understanding and mischaracterizations of Facets in Issue 438 for attaching to that issue. I will mention one issue with 438 here as it likely has bearing. 438 asserts a misunderstanding/mischaracterization that Facets are specifically designed and present to support Duck typing. This is incorrect. The design approach of Facet classes and their use with the core:hasFacet
property on UcoObjects is about far more than Duck typing or ObservableObjects in general. The Facet pattern has specific purpose that I will explain in comments on 438 but it predates the CASE (DFAX at the time) consensus requirement for Duck typing. Its prior existence simply offered a very straightforward way to support Duck typing when the requirement arose. Even if the CASE requirement for Duck typing were removed, Facets would still be required for UCO. I believe that Section 5 of the UCO Design Document explains the intent of Facets beyond Duck typing.
The issue here is simply to do with rdfs subclassing combined with SHACL property shapes and how they interact with subclassing hierarchies. There is NOTHING special about the core:Facet class (or subclasses) from the rdfs/owl/shacl perspective. It is only a little special in its level of abstraction and intended purpose for users. Facet is simply a class and all of the Facet subclasses are simply subclasses.
could just as easily be
The Facet classes simply apply to the core:hasFacet
property just like the vehicle classes could apply to a methodOfTransport
property.
There seems to be two potential issues raised here:
The first issue is very simple to address. While redundant property shape assertions are completely valid in SHACL they are not required and can simply be removed from the subclasses leaving only the shape at the top (ROOT) of the hierarchy. The large majority of users will be looking to the autogenerated documentation (rather than directly at the turtle) to understand UCO. This documentation clearly shows the relevant properties for each class including those "inherited" at each level of its position in a class hierarchy. So, with the property shape for addressValue asserted at the DigitalAddressFacet level it will still clearly be shown as a single occurrence when looking at the documentation for WiFiAddressFacet. The documentation also shows (right at the top of the page) where a particular class sits in a class hierarchy including not only superclasses but also subclasses which should make it straightforward for a user to find the most appropriate class for what they are trying to express. This documentation should mean that there is little potential for confusion on number 1 once redundant shapes are removed. Anyone looking directly at the turtle to understand UCO should not be confused by number 1 or number 2 above and if they are then all bets are off anyway as they will be confused by far bigger things than this.
If the properties are in the "Root" end, different problems may arise from UCO inability to enforce constraints at multiple different levels of the
Facet
subclass hierarchy.
I do not believe this is true. As long as the differential constraints are more rather than less restrictive as you descend the class hierarchy then there should be no problems and this is exactly as SHACL is designed to work.
Suppose
addressValue
was given a property shape inDigitalAddressFacet
, and that shape was not copied to the leaf nodes. Then supposeMACAddressFacet
defines an independent, supplementary property shape that constrainsaddressValue
to follow a certain regex (hex-byte, colon, hex-byte, colon, and so on). AWifiAddress
(notFacet
) instance could be defined like this and pass SHACL validation:
{
"@id": "kb:wifi-address-3",
"@type": "observable:WifiAddress",
"core:hasFacet": {
"@type": "observable:DigitalAddressFacet",
"observable:addressValue": "root@example.org"
}
}
This would pass with an email address being the value "of" a
WifiAddress
because the subclass hierarchy ofDigitalAddress
--MACAddress
--WifiAddress
is completely decoupled fromDigitalAddressFacet
--MACAddressFacet
--WifiAddressFacet
.
This decoupling is fundamentally necessary to support Duck typing. In Duck typing you may start with an ObservableObject object with one or more observable facet subclasses on it to characterize the details you know even if you are unsure yet what object(s) you are describing. You could then analyze what facets and properties are on the generic object and reach some conclusion as to what type of object or objects there are and then define these specific object types (e.g., MACAddress) and attach the relevant facet(s) on it. This is the entire concept of Duck typing. Of course, the current design and implementation in UCO does not require any user to follow Duck typing if it is not appropriate. If a user knows what type of object they are creating, they can simply create the object (e.g., WiFiAddress) with the appropriate facet(s) (e.g., WiFiAddressFacet) directly right from the start. I suspect the non-Duck typing approach will be used FAR more than the Duck typing approach but supporting Duck typing has been a fundamental requirement of CASE since its initiation.
The above json example would be perfectly legal if the propertyshape for addressValue at the DigitalAddressFacet provided no further constraint than xsd:string. And this is as it should be as it is the basis for RDFS subclassing and applying SHACL to it. The user of an email address string in a DigitalAddressFacet on a WiFiAddress ObservableObject would be a perfect example of a user drawing poor conclusions from a Duck typing situation where they looked at the email address string in the DigitalAddressFacet and thought "yeah, that looks like a WiFiAddress. They could have easily made better conclusions. That also does not mean that the user should not have used EmailAddressFacet rather than DigitalAddressFacet. This is a fundamental issue of users understanding subclassing and once again has NOTHING to do with Facets specifically. Using the vehicle analogy, if a user desired to express a Trip that used a 4-cylinder Toyota Camry they could easily express it as:
{
"@id": "kb:trip-1",
"@type": "Trip",
"methodOfTransport": {
"@type": "Vehicle",
"engineCylinderCount": 4
}
}
when it would be more appropriate to express as:
{
"@id": "kb:trip-1",
"@type": "Trip",
"methodOfTransport": {
"@type": "Sedan",
"engineCylinderCount": 4
}
}
If methodOfTransport
had a 0..* cardinality similar to core:hasFacet
It would also be completely possible for a user to express this as:
{
"@id": "kb:trip-1",
"@type": "Trip",
"methodOfTransport": [
{
"@type": "Vehicle",
"engineCylinderCount": 4
},
{
"@type": "Automobile",
"engineCylinderCount": 4
},
{
"@type": "Sedan",
"engineCylinderCount": 4
}
]
}
This is simply a matter of users understanding subclasses and the fact they should use the most appropriate one to their needs. This has NOTHING specifically to do with Facets.
I do not think we can do much about number 2 above. Understanding how subclassing works is a conceptual issue at a far broader scope than specifying UCO.
Net-Net: I believe this Issue misidentifies the underlying potential issues in play, very incorrectly asserts this as a problem specifically with Facet classes, and mischaracterizes the risks in play and potential solutions.
The core issues in play are that
@sbarnum I believe you and I are at an impasse.
I am not asserting UCO users would not understand subclassing. I am asserting that Facets are incompatible with subclassing for UcoObject
s, because of the parallel subclass hierarchy of Facet
s. I believe I have made a sufficient demonstration of usage patterns that would be problematic for UCO to enable, and that the correct solution is ultimately to remove Facet
s. Conceding to the release timeline to 1.0.0, I urge that guidance be added to documentation whenever Facet
sub-subclasses are used. (Edit: Sub-subclasses, not all subclasses.)
Once again, the "parallel" nature of UcoObjects such as observable:File and a facet of properties such as observable:FileFacet is only relevant to Duck typing and not to facets in general. And in reality the vast majority of observable facets to support Duck typing do NOT involve any "parallel" hierarchies. ContactFacet, DefinedEffectFacet and DigitalAddressFacet are the only such hierarchies. Duck typing utilizes the facet pattern but facets are useful and necessary beyond just Duck typing. Again, I will reserve the longer conversation about the misunderstanding of facets for a later date. On this specific proposal, I would not strongly object to it but would strongly assert if the solution chosen is only to add a textual caveat in class definitions then this must apply to all subclassing rather than only facet subclassing as I believe I have clearly shown with examples in my above comment. I would propose the far more appropriate solution to this problem is to remove redundant property shapes in subclasses and keep only the first one highest in the hierarchy. I believe this would be required to follow the basic "rules" we have been using for SHACL shapes anyway that we do not unnecessarily repeat shapes and that the only reason there should be a shape for the same property path deeper in a class hierarchy is if we are asserting some differential narrowing of constraints on it. There is no downside to this, it is simple, it adheres to how we have been handling shapes and it actually addresses the problem identified.
In response to discussion from Tuesday's meeting, this Issue is being re-scoped.
Several of the Facet
sub-subclasses repeat sh:PropertyShape
s wholly. This causes bugs with the documentation generation, and is part of the problem I'd raised in the proposal discussion.
See for example:
https://ontology.unifiedcyberontology.org/uco/documentation/class-observablewifiaddressfacet.html
addressValue
displays three times in the "Property shapes" table, due to each of three different subclass layers having an independent blank-node-identified sh:PropertyShape
specifying how that property must be used with that particular subclass. They are all completely redundant.
Requirement 1, as written, is being stricken. Requirement 2 is being put in as a replacement, accomplishing some of the same goals:
UCO must not wholly repeat property
sh:PropertyShape
s between subclasses - that is, define a completely matching set of SHACL constraints in a class C and subclass D.
Implementation happened to need to incorporate the resolution for PR 417 due to dropping constraints for one of the redundancies, so the merge of the resolution of this Issue has to wait for 417's merge (and also vote) to happen first.
Another light side-effect is I came across some Facet
subclasses that defined themselves as a subclass of core:Facet
and some other Facet
subclass. E.g. we had observable:PropertyReadEffectFacet rdfs:subClassOf core:Facet , observable:DefinedEffectFacet .
I've deleted those redundancies.
A less-light side-effect that came out of removing the redundancies is that there are now several new empty Facet
subclasses. Should they be deleted, in light of the Facet
trimming we are doing on the Device
subclass hierarchy proposal? The newly-propertyless Facet
s are:
observable:BluetoothAddressFacet
observable:EmailAddressFacet
observable:IPAddressFacet
observable:IPv4AddressFacet
observable:IPv6AddressFacet
observable:InstantMessagingAddressFacet
observable:MACAddressFacet
observable:SIPAddressFacet
observable:WifiAddressFacet
PR 454 has been filed to implement this proposal.
We have an outstanding question on whether the Facet
s caused to have no sh:PropertyShape
s should be deleted.
A piece of context to help us decide that - in the current state of develop
, before applying the effects of resolving Issue 445, these Facet
subclasses have no associated properties, and also are not superclasses of anything, per this SPARQL query:
SELECT ?nClass
WHERE {
?nClass rdfs:subClassOf+ core:Facet .
FILTER NOT EXISTS {
?nClass sh:property ?nPropertyShape .
}
FILTER NOT EXISTS {
?nSubClass rdfs:subClassOf ?nClass .
}
}
I would suggest that we strive to not add any NEW empty facets without a clear rationale to do so but that we do NOT remove any existing empty facets. There really is no risk to the empty facets other than potential noise. Let's try not to add any new unnecessary new noise but for existing empty facets (such as the identity facets) the minimal noise they may impart is likely acceptable for the placeholder framing they provide and removing them seems like more risk (losing their value) than keeping them (minimal noise).
I would suggest that we strive to not add any NEW empty facets without a clear rationale to do so but that we do NOT remove any existing empty facets. There really is no risk to the empty facets other than potential noise. Let's try not to add any new unnecessary new noise but for existing empty facets (such as the identity facets) the minimal noise they may impart is likely acceptable for the placeholder framing they provide and removing them seems like more risk (losing their value) than keeping them (minimal noise).
So, for the Facet
s caused to be empty by this proposal, we would retain them?
Yes
Note from OC Chair: The risk of this proposal is between low and moderate, so normally would not merit a Fast-Track review. However, the usage pattern being addressed may be more harmful than good to leave in 1.0.0. If OC members object to fast-tracking this, it will be staged as a 2.0.0 proposal.
Background
There are a few
core:Facet
subclasses that have a parent class between the class andcore:Facet
. One example isobservable:WifiAddressFacet
, an eventual subclass ofobservable:DigitalAddressFacet
. In that instance, and if I recall correctly some others as well, property shapes are defined redundantly onDigitalAddressFacet
,MACAddressFacet
, andWifiAddressFacet
. Their total definitions in today'sdevelop
branchobservable.ttl
are:The property shape for
observable:addressValue
is repeated across all three of those subclass layers. This creates a point of user confusion, potential incompatibility under RDFS expansion, and further potential incompatibility under OWL expansion.Take this snippet of data:
If a user performed RDFS expansion, the following classes would be added by merit of
rdfs:subClassOf
statements (spelled here in leaf-to-owl:Thing
class order):If a user is unaware of RDFS expansion, they might feel obligated to create all of the intermediary
Facet
s, and redundantly store the data:This would be a nuisance data behavior for UCO to require (causing much data bloat). It is also unnecessary because of something not yet encoded in UCO, but intended according to several conversations over the years, that there should only be one instance of class
XFacet
attached to each instance of classX
. This can be requested in a way with aowl:qualifiedCardinality 1
oncore:hasFacet
on eachUcoObject
subclass, but it would be a supreme nuisance to users to enforce in SHACL.Under RDFS expansion, we end up with three independent instances of
observable:DigitalAddressFacet
attached tokb:wifi-address-2
. This situation gets worse if a user doesn't realize the RDFS expansion, and further thinks they can use the extraaddressValue
slots in "Independent" parentFacet
classes to stash other data, like a second MAC Address on a two-NIC server:The
Facet
subclasses might be helpful for visualizing subclass hierarchies. However, the redundant storage of properties outside of the "Leaf"Facet
subclasses ends up, through the above illustrations, being harmful to users.These significantly confusing patterns are one of the motivations for doing away with
Facet
s altogether. However, in the event that Issue 438 does not pass, this proposal is offered for consideration.Requirements
For reasons explained in the Risks section, there is no technical solution possible while
Facet
s exist as a parallel hierarchical class structure to theUcoObject
class hierarchy.Requirement 1
Requirement 1 has been stricken. It was previously:
Requirement 2
UCO must not wholly repeat property sh:PropertyShapes between subclasses - that is, define a completely matching set of SHACL constraints in a class C and subclass D.
Risk / Benefit analysis
Benefits
Facet
subclass hierarchy, or even use theFacet
subclass hierarchy if there is no need.Facet
s, users that follow the guidance would be less likely to run into RDFS expansion issues that violate the not-yet-encoded design intent of "An instance ofUcoObject
classX
should have exactly oneXFacet
."Risks
The risks pertain to continued usage of
Facet
s, and not to any implementation changes due to this proposal.The risks stem from not being able to recommend de-duplication of properties on
Facet
s in UCO's current implementation. Putting all of the property shapes into just one "End" of theFacet
hierarchy has potential for confusion, whether the end is the leaf subclasses, or the "root" subclass.If the properties are in the "Leaf" end, it is difficult to encode in SHACL how to say what each leaf must have in terms of property shapes without inducing the usage confusion above. ("When do I create a
WifiAddressFacet
AND aDigitalAddressFacet
on one node?" will arise if users miss language in the class documentation comments.) This would be a breakage with most benefits of class hierarchies.If the properties are in the "Root" end, different problems may arise from UCO inability to enforce constraints at multiple different levels of the
Facet
subclass hierarchy. SupposeaddressValue
was given a property shape inDigitalAddressFacet
, and that shape was not copied to the leaf nodes. Then supposeMACAddressFacet
defines an independent, supplementary property shape that constrainsaddressValue
to follow a certain regex (hex-byte, colon, hex-byte, colon, and so on). AWifiAddress
(notFacet
) instance could be defined like this and pass SHACL validation:This would pass with an email address being the value "of" a
WifiAddress
because the subclass hierarchy ofDigitalAddress
--MACAddress
--WifiAddress
is completely decoupled fromDigitalAddressFacet
--MACAddressFacet
--WifiAddressFacet
.Last, it is not clear how to reconcile these issues with an object that is rightly one of the
UcoObject
classes corresponding to one of the intermediaryFacet
subclasses. (And likewise for any extension subclass anywhere along either class hierarchy.) If the correspondingFacet
has no property shapes, the user has no guidance or validation available for metadata meant to be attached to the object. This is also a reason that the intermediary subclass hierarchy ofFacet
s probably can't just be deleted outright.The proposer believes this is a design flaw with
Facet
s, and encourages further consideration of Issue 438. None of this proposal would be necessary if properties were directly onUcoObject
subclasses.Competencies demonstrated
The potential errors we wish to prevent are demonstrated in the background and risks.
Solution suggestion
Facet
subclasses. Append this string statement to their documentation strings, as a standalone line: "The most specific subclass of Facet should be used for the corresponding UcoObject. No parent class of this Facet should also be instantiated on this object."The multi-level
Facet
subclasses can be identified with this query:From UCO's current
develop
state (ba9da19
), these classes would need that annotation:Coordination
develop