Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Usage of core:Facet sub-subclasses creates RDFS-based confusion #445

Closed
8 tasks done
ajnelson-nist opened this issue Aug 11, 2022 · 8 comments · Fixed by #454
Closed
8 tasks done

Usage of core:Facet sub-subclasses creates RDFS-based confusion #445

ajnelson-nist opened this issue Aug 11, 2022 · 8 comments · Fixed by #454

Comments

@ajnelson-nist
Copy link
Contributor

ajnelson-nist commented Aug 11, 2022

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 and core:Facet. One example is observable:WifiAddressFacet, an eventual subclass of observable:DigitalAddressFacet. In that instance, and if I recall correctly some others as well, property shapes are defined redundantly on DigitalAddressFacet, MACAddressFacet, and WifiAddressFacet. Their total definitions in today's develop branch observable.ttl are:

observable:DigitalAddressFacet
	a
		owl:Class ,
		sh:NodeShape
		;
	rdfs:subClassOf core:Facet ;
	rdfs:label "DigitalAddressFacet"@en ;
	rdfs:comment "A digital address facet is a grouping of characteristics unique to an identifier assigned to enable routing and management of digital communication."@en ;
	sh:property
		[
			sh:datatype xsd:string ;
			sh:maxCount "1"^^xsd:integer ;
			sh:minCount "1"^^xsd:integer ;
			sh:nodeKind sh:Literal ;
			sh:path observable:addressValue ;
		] ,
		[
			sh:datatype xsd:string ;
			sh:maxCount "1"^^xsd:integer ;
			sh:nodeKind sh:Literal ;
			sh:path observable:displayName ;
		]
		;
	sh:targetClass observable:DigitalAddressFacet ;
	.

observable:MACAddressFacet
	a
		owl:Class ,
		sh:NodeShape
		;
	rdfs:subClassOf observable:DigitalAddressFacet ;
	rdfs:label "MACAddressFacet"@en ;
	rdfs:comment "A MAC address facet is a grouping of characteristics unique to a media access control standards conformant identifier assigned to a network interface to enable routing and management of communications at the data link layer of a network segment."@en ;
	sh:property [
		sh:datatype xsd:string ;
		sh:maxCount "1"^^xsd:integer ;
		sh:minCount "1"^^xsd:integer ;
		sh:nodeKind sh:Literal ;
		sh:path observable:addressValue ;
	] ;
	sh:targetClass observable:MACAddressFacet ;
	.

observable:WifiAddressFacet
	a
		owl:Class ,
		sh:NodeShape
		;
	rdfs:subClassOf observable:MACAddressFacet ;
	rdfs:label "WifiAddressFacet"@en ;
	rdfs:comment "A Wi-Fi address facet is a grouping of characteristics unique to a media access control (MAC) standards conformant identifier assigned to a device network interface to enable routing and management of IEEE 802.11 standards-conformant communications to and from that device."@en ;
	sh:property [
		sh:datatype xsd:string ;
		sh:maxCount "1"^^xsd:integer ;
		sh:minCount "1"^^xsd:integer ;
		sh:nodeKind sh:Literal ;
		sh:path observable:addressValue ;
	] ;
	sh:targetClass observable:WifiAddressFacet ;
	.

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:

{
    "@id": "kb:wifi-address-1",
    "@type": "observable:WifiAddress",
    "core:hasFacet": {
        "@type": "observable:WifiAddressFacet",
        "observable:addressValue": "01:23:45:67:89:ab"
    }
}

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

{
    "@id": "kb:wifi-address-1",
    "@type": [
        "observable:WifiAddress",
        "observable:MACAddress",
        "observable:DigitalAddress",
        "observable:ObservableObject",
        "(etc...)"
    ],
    "core:hasFacet": {
        "@type": [
             "observable:WifiAddressFacet"
            "observable:MACAddressFacet",
            "observable:DigitalAddressFacet",
            "core:Facet"
        ],
        "observable:addressValue": "01:23:45:67:89:ab"
    }
}

If a user is unaware of RDFS expansion, they might feel obligated to create all of the intermediary Facets, and redundantly store the data:

{
    "@id": "kb:wifi-address-2",
    "@type": "observable:WifiAddress",
    "core:hasFacet": [
        {
            "@type": "observable:DigitalAddressFacet",
            "observable:addressValue": "01:23:45:67:89:ab"
        },
        {
            "@type": "observable:MACAddressFacet",
            "observable:addressValue": "01:23:45:67:89:ab"
        },
        {
            "@type": "observable:WifiAddressFacet",
            "observable:addressValue": "01:23:45:67:89:ab"
        }
    ]
}

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 class X. This can be requested in a way with a owl:qualifiedCardinality 1 on core:hasFacet on each UcoObject 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 to kb:wifi-address-2. This situation gets worse if a user doesn't realize the RDFS expansion, and further thinks they can use the extra addressValue slots in "Independent" parent Facet classes to stash other data, like a second MAC Address on a two-NIC server:

{
    "@id": "kb:mac-address-1",
    "@type": "observable:MACAddress",
    "core:hasFacet": [
        {
            "@type": "observable:DigitalAddressFacet",
            "observable:addressValue": "cd:23:45:67:89:ab"
        },
        {
            "@type": "observable:MACAddressFacet",
            "observable:addressValue": "ef:98:76:54:32:10"
        }
    ]
}

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 Facets 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 Facets exist as a parallel hierarchical class structure to the UcoObject class hierarchy.

Requirement 1

Requirement 1 has been stricken. It was previously:

UCO must advise, in English documentation in every subclass of core:Facet that has a subclass, or that is a subclass of something aside from core:Facet, that the most specific Facet subclass be the only subclass to receive property-value assignments.

(If UCO did not have Facets, this requirement would be stricken, because properties on UcoObject subclasses would behave as with any other RDFS-based data model. Facets as graph individuals that serve as UcoObject property-housing proxies induce this requirement.)

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

  • Users should not feel forced to make redundant property assignments across the Facet subclass hierarchy, or even use the Facet subclass hierarchy if there is no need.
    • It is unfortunate that it seems the best protection mechanism seems to be natural-language advisory.
  • Lacking reasons to make the extra Facets, 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 of UcoObject class X should have exactly one XFacet."

Risks

The risks pertain to continued usage of Facets, and not to any implementation changes due to this proposal.

The risks stem from not being able to recommend de-duplication of properties on Facets in UCO's current implementation.
Putting all of the property shapes into just one "End" of the Facet 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 a DigitalAddressFacet 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. Suppose addressValue was given a property shape in DigitalAddressFacet, and that shape was not copied to the leaf nodes. Then suppose MACAddressFacet defines an independent, supplementary property shape that constrains addressValue to follow a certain regex (hex-byte, colon, hex-byte, colon, and so on). A WifiAddress (not Facet) 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": "[email protected]"
    }
}

This would pass with an email address being the value "of" a WifiAddress because the subclass hierarchy of DigitalAddress--MACAddress--WifiAddress is completely decoupled from DigitalAddressFacet--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 intermediary Facet subclasses. (And likewise for any extension subclass anywhere along either class hierarchy.) If the corresponding Facet 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 of Facets probably can't just be deleted outright.

The proposer believes this is a design flaw with Facets, and encourages further consideration of Issue 438. None of this proposal would be necessary if properties were directly on UcoObject subclasses.

Competencies demonstrated

The potential errors we wish to prevent are demonstrated in the background and risks.

Solution suggestion

  • Identify all multi-level 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:

SELECT DISTINCT ?nClass
WHERE {
  {
    ?nClass rdfs:subClassOf/rdfs:subClassOf+ core:Facet .
  }
  UNION
  {
    ?nClass rdfs:subClassOf core:Facet .
    ?nSubClass rdfs:subClassOf ?nClass .
  }
}
ORDER BY ?nClass

From UCO's current develop state (ba9da19), these classes would need that annotation:

?nClass
0 https://ontology.unifiedcyberontology.org/uco/identity/AddressFacet
1 https://ontology.unifiedcyberontology.org/uco/identity/AffiliationFacet
2 https://ontology.unifiedcyberontology.org/uco/identity/BirthInformationFacet
3 https://ontology.unifiedcyberontology.org/uco/identity/CountryOfResidenceFacet
4 https://ontology.unifiedcyberontology.org/uco/identity/EventsFacet
5 https://ontology.unifiedcyberontology.org/uco/identity/IdentifierFacet
6 https://ontology.unifiedcyberontology.org/uco/identity/IdentityFacet
7 https://ontology.unifiedcyberontology.org/uco/identity/LanguagesFacet
8 https://ontology.unifiedcyberontology.org/uco/identity/NationalityFacet
9 https://ontology.unifiedcyberontology.org/uco/identity/OccupationFacet
10 https://ontology.unifiedcyberontology.org/uco/identity/OrganizationDetailsFacet
11 https://ontology.unifiedcyberontology.org/uco/identity/PersonalDetailsFacet
12 https://ontology.unifiedcyberontology.org/uco/identity/PhysicalInfoFacet
13 https://ontology.unifiedcyberontology.org/uco/identity/QualificationFacet
14 https://ontology.unifiedcyberontology.org/uco/identity/RelatedIdentityFacet
15 https://ontology.unifiedcyberontology.org/uco/identity/SimpleNameFacet
16 https://ontology.unifiedcyberontology.org/uco/identity/VisaFacet
17 https://ontology.unifiedcyberontology.org/uco/observable/BluetoothAddressFacet
18 https://ontology.unifiedcyberontology.org/uco/observable/ContactFacet
19 https://ontology.unifiedcyberontology.org/uco/observable/DefinedEffectFacet
20 https://ontology.unifiedcyberontology.org/uco/observable/DigitalAddressFacet
21 https://ontology.unifiedcyberontology.org/uco/observable/EmailAddressFacet
22 https://ontology.unifiedcyberontology.org/uco/observable/IPAddressFacet
23 https://ontology.unifiedcyberontology.org/uco/observable/IPv4AddressFacet
24 https://ontology.unifiedcyberontology.org/uco/observable/IPv6AddressFacet
25 https://ontology.unifiedcyberontology.org/uco/observable/InstantMessagingAddressFacet
26 https://ontology.unifiedcyberontology.org/uco/observable/MACAddressFacet
27 https://ontology.unifiedcyberontology.org/uco/observable/PropertiesEnumeratedEffectFacet
28 https://ontology.unifiedcyberontology.org/uco/observable/PropertyReadEffectFacet
29 https://ontology.unifiedcyberontology.org/uco/observable/SIPAddressFacet
30 https://ontology.unifiedcyberontology.org/uco/observable/SendControlCodeEffectFacet
31 https://ontology.unifiedcyberontology.org/uco/observable/StateChangeEffectFacet
32 https://ontology.unifiedcyberontology.org/uco/observable/ValuesEnumeratedEffectFacet
33 https://ontology.unifiedcyberontology.org/uco/observable/WhoisContactFacet
34 https://ontology.unifiedcyberontology.org/uco/observable/WifiAddressFacet

Coordination

  • Tracking in Jira ticket OC-97
  • Solution announced to OCs on 2022-08-10
  • Solutions Approval to be discussed in OC meeting, 2022-08-16
  • Solutions Approval to be discussed again in OC meeting, 2022-08-25
  • Solutions Approval vote occurred, passing, on 2022-08-25
  • Solutions development phase completed.
  • Implementation merged into develop
  • Milestone linked
  • Documentation logged in pending release page
@sbarnum
Copy link
Contributor

sbarnum commented Aug 11, 2022

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.

  • DigitalAddressFacet
    • MACAddressFacet
      • WiFiAddressFacet

could just as easily be

  • Vehicle
    • Automobile
      • Sedan

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:

  1. Users could potentially be confused by redundant property shape assertions at multiple levels of a class hierarchy (once again NOTHING specifically to do with Facet)
  2. Users could potentially be confused by how subclassing works in general

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 in DigitalAddressFacet, and that shape was not copied to the leaf nodes. Then suppose MACAddressFacet defines an independent, supplementary property shape that constrains addressValue to follow a certain regex (hex-byte, colon, hex-byte, colon, and so on). A WifiAddress (not Facet) 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": "[email protected]"
    }
}

This would pass with an email address being the value "of" a WifiAddress because the subclass hierarchy of DigitalAddress--MACAddress--WifiAddress is completely decoupled from DigitalAddressFacet--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

  1. Redundant SHACL property shapes in a class hierarchy could cause significant use confusion. I personally do not believe that there is significant risk of this but the issue is simple to address by removing the redundant shape assertions leaving only the first one highest in the hierarchy
  2. Users may be confused by how subclasses work and choose a less than optimal level in a given subclass hierarchy. I do not think that this is an issue for UCO to attempt to resolve as it is much broader than UCO. If you wish to mitigate the risk of a lacking understanding of subclassing and duplicatively asserting multiple classes with the same property within a class hierarchy (as shown in the last example above) you could add the sort of textual caveat proposed in the Solution Suggestion above but it would need to be done for ALL subclasses not just for Facet subclasses as this is a general subclassing issue that is NOT specific to Facets. I personally do not think this is a good idea. It itself is likely to cause as much or more confusion than it solves and sets a poor precedent for adding such extraneous detail to our definitions to cover a users potential inadequate understanding of modeling concepts.

@ajnelson-nist
Copy link
Contributor Author

ajnelson-nist commented Aug 12, 2022

@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 UcoObjects, because of the parallel subclass hierarchy of Facets. 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 Facets. 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.)

@sbarnum
Copy link
Contributor

sbarnum commented Aug 15, 2022

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.

@ajnelson-nist
Copy link
Contributor Author

In response to discussion from Tuesday's meeting, this Issue is being re-scoped.

Several of the Facet sub-subclasses repeat sh:PropertyShapes 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:PropertyShapes 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 Facets are:

  • observable:BluetoothAddressFacet
  • observable:EmailAddressFacet
  • observable:IPAddressFacet
  • observable:IPv4AddressFacet
  • observable:IPv6AddressFacet
  • observable:InstantMessagingAddressFacet
  • observable:MACAddressFacet
  • observable:SIPAddressFacet
  • observable:WifiAddressFacet

@ajnelson-nist ajnelson-nist added this to the UCO 1.0.0 milestone Aug 18, 2022
ajnelson-nist added a commit that referenced this issue Aug 18, 2022
These property shapes were identified for removal by visual review of
the results of this query:

```sparql
SELECT DISTINCT ?nClass
WHERE {
  {
    ?nClass rdfs:subClassOf/rdfs:subClassOf+ core:Facet .
  }
  UNION
  {
    ?nClass rdfs:subClassOf core:Facet .
    ?nSubClass rdfs:subClassOf ?nClass .
  }
}
ORDER BY ?nClass
```

The same shapes were identified for removal by visual review of the results of this query:

```sparql
SELECT DISTINCT ?nClass ?nSubClass ?nProperty
WHERE {
  ?nClass
    rdfs:subClassOf* core:Facet ;
    sh:property/sh:path ?nProperty ;
    .

  ?nSubClass
    rdfs:subClassOf+ ?nClass ;
    sh:property/sh:path ?nProperty ;
    .
}
ORDER BY ?nClass ?nSubClass ?nProperty
```

References:
* #445

Signed-off-by: Alex Nelson <[email protected]>
ajnelson-nist added a commit that referenced this issue Aug 18, 2022
These property shapes were identified by visual review of the results of
this query:

```sparql
SELECT DISTINCT ?nClass ?nSubClass ?nProperty
WHERE {
  ?nClass
    sh:property/sh:path ?nProperty ;
    .

  ?nSubClass
    rdfs:subClassOf+ ?nClass ;
    sh:property/sh:path ?nProperty ;
    .
}
ORDER BY ?nClass
```

(The difference since the last patch is `?nClass` is no longer tied to
`Facet`s.)

This patch alone will trigger a CI failure from the SHIR code base, due
to SHIR 0.2.0 flagging dropped constraints as errors.  A follow-on patch
will merge in the resolution to PR 417 in order to resolve this bug
without needing to work through addressing semi-open vocabulary issues
when subclasses are involved.  Incidentally, Issue 442 is now mooted.

References:
* #417
* #442
* #445

Signed-off-by: Alex Nelson <[email protected]>
@ajnelson-nist ajnelson-nist linked a pull request Aug 18, 2022 that will close this issue
11 tasks
ajnelson-nist added a commit that referenced this issue Aug 18, 2022
References:
* #445

Signed-off-by: Alex Nelson <[email protected]>
@ajnelson-nist
Copy link
Contributor Author

PR 454 has been filed to implement this proposal.

We have an outstanding question on whether the Facets caused to have no sh:PropertyShapes 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 .
  }
}
?nClass
0 https://ontology.unifiedcyberontology.org/uco/identity/AffiliationFacet
1 https://ontology.unifiedcyberontology.org/uco/identity/CountryOfResidenceFacet
2 https://ontology.unifiedcyberontology.org/uco/identity/EventsFacet
3 https://ontology.unifiedcyberontology.org/uco/identity/IdentifierFacet
4 https://ontology.unifiedcyberontology.org/uco/identity/LanguagesFacet
5 https://ontology.unifiedcyberontology.org/uco/identity/NationalityFacet
6 https://ontology.unifiedcyberontology.org/uco/identity/OccupationFacet
7 https://ontology.unifiedcyberontology.org/uco/identity/OrganizationDetailsFacet
8 https://ontology.unifiedcyberontology.org/uco/identity/PersonalDetailsFacet
9 https://ontology.unifiedcyberontology.org/uco/identity/PhysicalInfoFacet
10 https://ontology.unifiedcyberontology.org/uco/identity/QualificationFacet
11 https://ontology.unifiedcyberontology.org/uco/identity/RelatedIdentityFacet
12 https://ontology.unifiedcyberontology.org/uco/identity/VisaFacet
13 https://ontology.unifiedcyberontology.org/uco/observable/NTFSFilePermissionsFacet
14 https://ontology.unifiedcyberontology.org/uco/observable/UNIXFilePermissionsFacet

@sbarnum
Copy link
Contributor

sbarnum commented Aug 18, 2022

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

@ajnelson-nist
Copy link
Contributor Author

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 Facets caused to be empty by this proposal, we would retain them?

@sbarnum
Copy link
Contributor

sbarnum commented Aug 25, 2022

Yes

ajnelson-nist added a commit to casework/CASE-Archive that referenced this issue Aug 29, 2022
No effects were observed on Make-managed files.

References:
* ucoProject/UCO#445

Signed-off-by: Alex Nelson <[email protected]>
ajnelson-nist added a commit to casework/CASE-Examples that referenced this issue Aug 29, 2022
No effects were observed on Make-managed files.

References:
* ucoProject/UCO#445

Signed-off-by: Alex Nelson <[email protected]>
ajnelson-nist added a commit to casework/casework.github.io that referenced this issue Aug 29, 2022
A follow-on patch will regenerate Make-managed files.

References:
* ucoProject/UCO#445

Signed-off-by: Alex Nelson <[email protected]>
ajnelson-nist added a commit to casework/casework.github.io that referenced this issue Aug 29, 2022
References:
* ucoProject/UCO#445

Signed-off-by: Alex Nelson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants