Azure / opendigitaltwins-building

Open Digital Twins Definition Language (DTDL) RealEstateCore Ontology
MIT License
150 stars 42 forks source link

CapabilityTagSet added #46

Closed hammar closed 3 years ago

hammar commented 3 years ago

Rick, Alina: is this in line with your requirements?

Rick: note that I did not incorporate phenomenon/position in this tag structure, rather, I extended the existing generic properties on capability.

hammar commented 3 years ago

Note: this is possibly incomplete, I lifted the design from the Willow models, but as I recall some additional tags were needed, for, e.g., markers, stage, etc.

hammar commented 3 years ago

Hi,

I will look at the open matters later tonight CET or tomorrow:

  1. Re. phenomenon and position: suggest these stay on Capability for the time being, to reduce upstream OWL model impact. Would that be acceptable?
  2. I will remove capability prefix on the tag properties.
  3. Re. the externalIds thing, @alinamstanciu, that seems orthogonal to this PR. For which relationships in the ontology should this be a thing? Surely not ALL of them? But if a subset, then which?
  4. Will copy stage/effective/demand from Willow models
  5. Custom (user defined) marker tags: adds a bit of complexity to translation code. Could we push this forward until there is a verified need?

Karl

rszcodronski commented 3 years ago

Hi Karl,

I feel pretty strongly that phenomenon and position belong with the others. If they are treated differently, this will be very difficult to explain.

Agree with all of your other replies.

hammar commented 3 years ago

OK, I'll check position/phenomenon w/ upstream REC colleagues and see what I can get everyone on board with. In the interim proceeding with the other choices.

Could I ask for some descriptive texts for the tag properties, e.g., "demand", "effective", etc.? A sentence for each of these new properties would be sufficient. To be used w/ rdfs:comment and DTDL description tags, respectively? I think it would be useful for clarifying their intended usage to some categories of users.

rszcodronski commented 3 years ago

effective: Current control setpoint in effect taking into account other factors (Project Haystack)

demand: Rate required for a process. For a setpoint, this sets the required rate for a process such as cooling, heating, air flow, or water flow. For a sensor, this measures the rate of a process over a given interval such as electrical power demand or cooling energy demand.

stage: Stage number of a control loop for an equipment, equipment group, or system that is defined by the process controller. The first stage is 1, second stage 2, etc. (Project Haystack)

assetComponent: The component that is part of an asset which the capability is associated with.

hvacMode: Operational mode for HVAC equipment (Project Haystack)

occupancyMode: Mode which defines how to control a space based on occupancy status

electricalPhase: Phase measurement in a three-phase electrical system (Project Haystack)

limit: Parameter that places and upper (maximum) or lower (minimum) bound on permitted values of another capability.

alinamstanciu commented 3 years ago

@hammar and @rszcodronski , could we please close on this, we are behind with this delivery? Thanks a lot!

hammar commented 3 years ago

@hammar and @rszcodronski , could we please close on this, we are behind with this delivery? Thanks a lot!

If I don’t hear back from Erik/Per on this by Monday I’ll proceed w/ moving phenomenon/placement as suggested by Rick (Friday is a public holiday and Thursday is a de-facto holiday for many/most white collar workers).

Beside that, all that is missing is to add the comments that Rick provided (thanks!) and to get clarification on your comment re. the assetComponent tag property (did you oppose this?) and the external IDs (the latter could be punted).

rszcodronski commented 3 years ago

@alinamstanciu - I think your "i would say no comment" on assetComponent was an error? I assume we are keeping this.

alinamstanciu commented 3 years ago

@alinamstanciu - I think your "i would say no comment" on assetComponent was an error? I assume we are keeping this.

No, it wasn't an error, I was responding to your comment on having "capability" as a prefix which is not desired. @rszcodronski , could you please send Karl the latest definitions so we can include them here or are we doing a new PR for that?

In order to create a new version 1.1, we need to have definitions which I've done 80%, Rick you were supposed to finish them.

Thanks

rszcodronski commented 3 years ago

Ok, yes I think we are all saying the same thing in different ways:

  1. Karl has removed the "capability" prefix on all of these tag properties.
  2. All of the tags properties that were in the original commit will remain
  3. Pending Idun feedback, phenomenon and position will move into the tags to sit alongside the other tags.

@alina - Let's get this PR merged and then we can do the dtmi descriptions. I started but have not been able to finish.

hammar commented 3 years ago

Ok, yes I think we are all saying the same thing in different ways:

  1. Karl has removed the "capability" prefix on all of these tag properties.
  2. All of the tags properties that were in the original commit will remain
  3. Pending Idun feedback, phenomenon and position will move into the tags to sit alongside the other tags.

@alina - Let's get this PR merged and then we can do the dtmi descriptions. I started but have not been able to finish.

I got go-ahead from REC-OWL and will proceed with all of this on Sunday or early Monday CEST. Now time for the traditional Midsummer celebrations. Have a great weekend!