Skip to content

Conversation

MatthewSteen
Copy link
Member

@MatthewSteen MatthewSteen commented Jul 4, 2024

Clean up for #263.

@MatthewSteen MatthewSteen marked this pull request as draft August 1, 2024 20:42
@MatthewSteen MatthewSteen marked this pull request as ready for review August 21, 2024 15:09
Comment on lines +29 to +36
(_:relief rdf:type rdf:Alt
rdf:_1
p:relief-damper
brick:hasPoint p:bldg-static-pressure
rdf:_2
p:relief-fan
p:relief-fan-damper
brick:hasPoint p:bldg-static-pressure ),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the separate template in the original file, which didn't appear to work as intended in G36 because it's optional.

# compose on the ahu's 'name'
with-relief-damper:
body: >
@prefix p: <urn:___param___#> .
@prefix brick: <https://brickschema.org/schema/Brick#> .
p:name brick:hasPart p:relief-fan, p:relief-damper ;
brick:hasPoint p:sp-sensor .
p:relief-damper a brick:Exhaust_Damper .
optional: ["sp-sensor"]
dependencies:
- template: relief-fan
args: {"name": "relief-fan"}
- template: damper
args: {"name": "relief-damper"}
- template: https://brickschema.org/schema/Brick#Static_Pressure_Sensor
library: https://brickschema.org/schema/1.4/Brick
args: {"name": "sp-sensor"}

Comment on lines +37 to +44
( p:return-fan
brick:hasPoint
p:return-fan-reset
p:return-fan-sa-flow
p:return-fan-ra-flow
p:return-fan-pressure
p:bldg-static-pressure
p:return-fan-exhaust-damper ) ;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the separate template in the original file, which didn't appear to work as intended in G36 because it's optional.

with-return-fan:
body: >
@prefix p: <urn:___param___#> .
@prefix brick: <https://brickschema.org/schema/Brick#> .
p:name brick:hasPart p:return-fan ;
brick:hasPoint p:supply-air-flow, p:return-air-flow, p:sp-sensor .
p:relief-damper a brick:Exhaust_Damper .
optional: ["supply-air-flow", "return-air-flow", "sp-sensor"]
dependencies:
- template: return-fan
args: {"name": "return-fan"}
- template: damper
args: {"name": "relief-damper"}
- template: https://brickschema.org/schema/Brick#Static_Pressure_Sensor
library: https://brickschema.org/schema/1.4/Brick
args: {"name": "sp-sensor"}
- template: https://brickschema.org/schema/Brick#Supply_Air_Flow_Sensor
library: https://brickschema.org/schema/1.4/Brick
args: {"name": "supply-air-flow"}
- template: https://brickschema.org/schema/Brick#Return_Air_Flow_Sensor
library: https://brickschema.org/schema/1.4/Brick
args: {"name": "return-air-flow"}

p:name a brick:Exhaust_Fan ;
brick:hasPoint p:start_stop, p:status, p:zat .
optional: ["zat"]
p:name a brick:Fan ;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to make the object here a brick:Fan class instead of p:name a p:constant-speed-fan (from components.yml like the TTL file) because it failed test_library.py. There are a couple/few cases where this may be convenient and might be worth revisiting during a future template discussion/refactor.

cls = <class 'buildingmotif.dataclasses.library._template_dependency'>, d = {'args': {'name': 'cav-fan'}}, dependent_library_name = 'guideline36'

    @classmethod
    def from_dict(
        cls, d: Dict[str, Any], dependent_library_name: str
    ) -> "_template_dependency":
        """Creates a py:class:`_template_dependency` from a dictionary.

        :param d: dictionary
        :type d: Dict[str, Any]
        :param dependent_library_name: library name
        :type dependent_library_name: str
        :return: the _template_dependency from the dict
        :rtype: _template_dependency
        """
>       template_name = d["template"]
E       KeyError: 'template'

buildingmotif/dataclasses/library.py:63: KeyError

@MatthewSteen MatthewSteen requested a review from gtfierro August 21, 2024 15:52
@MatthewSteen
Copy link
Member Author

@gtfierro this is ready. Please specifically review the template body syntax.

I think the CI test failure is unrelated and will fix in the guideline36-section4 branch if still present.

FAILED tests/unit/api/test_model.py::test_test_model_against_shapes[pyshacl] - sqlalchemy.exc.NoResultFound: No template found with name https://brickschema.org/schema/Brick#Leaving_Chilled_Water_Flow_Sensor
FAILED tests/unit/api/test_model.py::test_test_model_against_shapes[topquadrant] - sqlalchemy.exc.NoResultFound: No template found with name https://brickschema.org/schema/Brick#Leaving_Chilled_Water_Flow_Sensor

@MatthewSteen MatthewSteen modified the milestone: 0.3.0 Sep 9, 2024
@gtfierro gtfierro added this to the 0.3.0 milestone Sep 9, 2024
@gtfierro
Copy link
Collaborator

@MatthewSteen I've gone through the templates and I think they look good. The rdf:Alt syntax will eventually need to be supported in buildingMOTIF but I have no other comments. I'm running tests locally on these to verify everything works.

Some of the missing classes like the Leaving_Chilled_Water_Flow_Sensor are probably failing because the copy of Brick is old (v1.4.1 of Brick has this class, for example: https://ontology.brickschema.org/brick/Leaving_Chilled_Water_Flow_Sensor.html).

@MatthewSteen
Copy link
Member Author

@gtfierro thanks. Should we update Brick to 1.4 in a separate PR? I've updated the templates here and will do the same in the base branch.

@gtfierro
Copy link
Collaborator

@gtfierro thanks. Should we update Brick to 1.4 in a separate PR? I've updated the templates here and will do the same in the base branch.

Yes, I think that's the right move. I'll handle that momentarily

@MatthewSteen
Copy link
Member Author

@gtfierro are you ok with merging this into the base G36 branch?

@gtfierro
Copy link
Collaborator

Yes! Sounds good to me. I can help fix some other validation issues this weekend

@MatthewSteen
Copy link
Member Author

Great! I'll merge and finalize in the base G36 branch.

@MatthewSteen MatthewSteen merged commit 7c41159 into guideline36-section4 Sep 13, 2024
2 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants