Fixing runtime issues in Jupyter notebook (#348)

* move the target query out of the hot loop; we probably do not need to compute the cbd because we are already passing in the ontology graph * reduce the number of shapes we check in the notebook * fix composition of shacl shapes * break out integration tests to only run once * remove commented line * break out libraries into its own test * remove libraries from integration * try to run library tests on approval/merge * use new model * try more triggers * make only library tests run when event type is not push * remove style dependency for library tests --------- Co-authored-by: TShapinsky <[email protected]>
NREL · Sep 16, 2024 · e24d741 · e24d741
1 parent 471ef84
commit e24d741
Show file tree

Hide file tree

Showing 5 changed files with 92 additions and 29 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -1,7 +1,9 @@
 name: continuous integration
 
 on:
-  push
+  push:
+  pull_request_review:
+    types: [submitted]
 
 concurrency:
   group: ${{ github.workflow }}-${{ github.ref }}
@@ -10,6 +12,7 @@ concurrency:
 jobs:
 
   styling:
+    if: github.event_name == 'push'
     runs-on: ubuntu-latest
     steps:
       - name: checkout
@@ -31,7 +34,10 @@ jobs:
           poetry run isort . --check
           poetry run black . --check
 
+  # The "testing" job verifies the base SDK functionality across
+  # all supported Python versions.
   testing:
+    if: github.event_name == 'push'
     needs: styling
     runs-on: ubuntu-latest
     strategy:
@@ -62,19 +68,79 @@ jobs:
         run: poetry run mypy --ignore-missing-imports
       - name: unit tests
         run: poetry run pytest tests/unit --cov=./ --cov-report=xml
+      - name: build tests
+        run: poetry build
+
+  # We only run "integration" tests on the latest Python version.
+  # These tests detect if changes to ontologies, libraries, models and BuildingMOTIF
+  # affect correct operation of notebooks and BACnet scans. Library integration testing
+  # is a separate job
+  integration:
+    if: github.event_name == 'push'
+    needs: styling
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        python-version: ['3.11']
+    steps:
+      - name: checkout
+        uses: actions/checkout@v4
+      - uses: actions/setup-java@v4 # for topquadrant shacl support
+        with:
+          distribution: 'temurin'
+          java-version: '21'
+      - name: setup-python
+        uses: actions/setup-python@v5
+        with:
+          python-version: ${{ matrix.python-version }}
+      - name: install-poetry
+        uses: snok/install-poetry@v1
+        with:
+          version: 1.4.0
+          virtualenvs-in-project: false
+          virtualenvs-path: ~/.virtualenvs
+      - name: poetry install
+        run: poetry install --all-extras
       - name: integration tests
         run: poetry run pytest tests/integration
-      - name: library tests
-        run: poetry run pytest tests/library
       - name: bacnet tests
         run: |
           cd tests/integration/fixtures/bacnet
           docker compose build device buildingmotif
           docker compose run -d device
           docker compose run buildingmotif poetry run pytest -m bacnet
           docker compose down
-      - name: build tests
-        run: poetry build
+
+  # We only run "library" tests on the latest Python version.
+  # These tests detect if changes to ontologies, libraries, models and BuildingMOTIF
+  # affect correct operation of templates, shapes, and validation
+  libraries:
+    if: github.event.review.state == 'approved' || github.ref == 'refs/heads/main' || github.ref == 'refs/heads/develop'
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        python-version: ['3.11']
+    steps:
+      - name: checkout
+        uses: actions/checkout@v4
+      - uses: actions/setup-java@v4 # for topquadrant shacl support
+        with:
+          distribution: 'temurin'
+          java-version: '21'
+      - name: setup-python
+        uses: actions/setup-python@v5
+        with:
+          python-version: ${{ matrix.python-version }}
+      - name: install-poetry
+        uses: snok/install-poetry@v1
+        with:
+          version: 1.4.0
+          virtualenvs-in-project: false
+          virtualenvs-path: ~/.virtualenvs
+      - name: poetry install
+        run: poetry install --all-extras
+      - name: library tests
+        run: poetry run pytest tests/library
 
   coverage:
     needs: testing

diff --git a/buildingmotif/dataclasses/model.py b/buildingmotif/dataclasses/model.py
@@ -256,28 +256,26 @@ def test_model_against_shapes(
 
         results = {}
 
+        targets = model_graph.query(
+            f"""
+            PREFIX rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#>
+            PREFIX rdfs: <http://www.w3.org/2000/01/rdf-schema#>
+            SELECT ?target
+            WHERE {{
+                ?target rdf:type/rdfs:subClassOf* <{target_class}>
+
+            }}
+        """
+        )
+        # skolemize the shape graph so we have consistent identifiers across
+        # validation through the interpretation of the validation report
+        ontology_graph = ontology_graph.skolemize()
+
         for shape_uri in shapes_to_test:
-            targets = model_graph.query(
-                f"""
-                PREFIX rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#>
-                PREFIX rdfs: <http://www.w3.org/2000/01/rdf-schema#>
-                SELECT ?target
-                WHERE {{
-                    ?target rdf:type/rdfs:subClassOf* <{target_class}>
-
-                }}
-            """
-            )
             temp_model_graph = copy_graph(model_graph)
             for (s,) in targets:
                 temp_model_graph.add((URIRef(s), A, shape_uri))
 
-            temp_model_graph += ontology_graph.cbd(shape_uri)
-
-            # skolemize the shape graph so we have consistent identifiers across
-            # validation through the interpretation of the validation report
-            ontology_graph = ontology_graph.skolemize()
-
             valid, report_g, report_str = shacl_validate(
                 temp_model_graph, ontology_graph, engine=self._bm.shacl_engine
             )

diff --git a/libraries/ashrae/guideline36/5.16.14-multiple-zone-vav-ahu-afdd.ttl b/libraries/ashrae/guideline36/5.16.14-multiple-zone-vav-ahu-afdd.ttl
@@ -133,11 +133,8 @@
           sh:qualifiedMinCount 1 ;
         ]
     ) ;
-    sh:property [
-        sh:path brick:hasPoint ;
-        sh:qualifiedValueShape [ sh:node :percent-oa-minimum ] ;
-        sh:qualifiedMinCount 1 ;
-    ] .
+    # use subclassof to compose shapes
+    rdfs:subClassOf :percent-oa-minimum .
 
 :percent-oa-fraction a sh:NodeShape, owl:Class ;
     rdfs:label "%OA" ;

diff --git a/notebooks/223P-Validation.ipynb b/notebooks/223P-Validation.ipynb
@@ -140,7 +140,7 @@
    "source": [
     "# create a model to hold an example building from models.open223.info\n",
     "lbnl_example_building = Model.create(Namespace(\"urn:lbnl\"))\n",
-    "lbnl_example_building.graph.parse(\"https://models.open223.info/compiled/lbnl-example.ttl\")"
+    "lbnl_example_building.graph.parse(\"https://models.open223.info/compiled/lbnl-bdg3-1.ttl\")"
    ]
   },
   {

diff --git a/notebooks/Existing-model-validation-example.ipynb b/notebooks/Existing-model-validation-example.ipynb
@@ -744,7 +744,9 @@
    "outputs": [],
    "source": [
     "shape_collections = [brick.get_shape_collection(), ashrae_g36.get_shape_collection()]\n",
-    "shapes_to_test = ashrae_g36.get_shape_collection().get_shapes_of_definition_type(BMOTIF[\"Analytics_Application\"])\n"
+    "shapes_to_test = ashrae_g36.get_shape_collection().get_shapes_of_definition_type(BMOTIF[\"Analytics_Application\"])\n",
+    "# choose the first 2 shapes to test; running them all can take a few minutes\n",
+    "shapes_to_test = shapes_to_test[:2]"
    ]
   },
   {