From e38cbc8fa67246d0d33cc2e85af9a77695ff9696 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Thu, 27 Aug 2026 16:54:39 +0200 Subject: [PATCH 1/8] Tweak MVD apply to retain one row per template match --- .../checks/statistics/apply_mvd.py | 24 +-- .../tests/test_template_statistics.py | 58 ++++-- .../ifc_validation/tests_statistics_query.py | 190 +++++++++++++++++- 3 files changed, 243 insertions(+), 29 deletions(-) diff --git a/backend/apps/ifc_validation/checks/statistics/apply_mvd.py b/backend/apps/ifc_validation/checks/statistics/apply_mvd.py index fe53d5d4..20769f81 100644 --- a/backend/apps/ifc_validation/checks/statistics/apply_mvd.py +++ b/backend/apps/ifc_validation/checks/statistics/apply_mvd.py @@ -51,19 +51,17 @@ def extract_template_statistics( for focus in focus_instances: rows = concept.extract(focus) - if not rows: - continue - graph = { - concept.binding_for(key) or key.attribute: json_value(value) - for row in rows - for key, value in row.items() - } - results.append({ - "template": markdown.name, - "focus_step_id": focus.id(), - "focus_ifc_type": focus.is_a(), - "graph": graph, - }) + for row in rows: + graph = { + concept.binding_for(key) or key.attribute: json_value(value) + for key, value in row.items() + } + results.append({ + "template": markdown.name, + "focus_step_id": focus.id(), + "focus_ifc_type": focus.is_a(), + "graph": graph, + }) return results diff --git a/backend/apps/ifc_validation/checks/statistics/tests/test_template_statistics.py b/backend/apps/ifc_validation/checks/statistics/tests/test_template_statistics.py index ad363903..5023930f 100644 --- a/backend/apps/ifc_validation/checks/statistics/tests/test_template_statistics.py +++ b/backend/apps/ifc_validation/checks/statistics/tests/test_template_statistics.py @@ -13,27 +13,57 @@ def test_column_property_set_statistics(): results = extract_template_statistics(FIXTURES / "ColumnPSetsOfSets.ifc") - assert len(results) == 17 - assert {result["template"] for result in results} == {"Use_of_property_types.md"} - assert {result["focus_ifc_type"] for result in results} == {"IfcPropertySet"} - assert { - result["graph"]["PropertyType"] for result in results - } == {"IfcPropertySingleValue"} - assert Counter( - result["graph"]["PropertySetName"] for result in results - ) == Counter({ - "Pset_SpaceCommon": 4, - "Pset_BuildingStoreyCommon": 3, - "Pset_ColumnCommon": 2, + property_type_results = [ + result + for result in results + if result["template"] == "Use_of_property_types.md" + ] + definition_set_results = [ + result + for result in results + if result["template"] == "Usage_of_IfcPropertySetDefinitionSet.md" + ] + expected_property_projection_names = Counter({ + "Pset_SpaceCommon": 8, + "Pset_BuildingStoreyCommon": 6, + "Pset_ColumnCommon": 4, "Pset_SiteCommon": 1, "Pset_EnvironmentalImpactIndicators": 1, "Pset_ReinforcementBarPitchOfColumn": 1, - "Pset_BuildingCommon": 1, - "Pset_BuildingElementProxyCommon": 1, + "Pset_BuildingCommon": 3, + "Pset_BuildingElementProxyCommon": 2, "Pset_BuildingSystemCommon": 1, "PSet_1": 1, "PSet_2": 1, }) + expected_definition_set_results = [{ + "template": "Usage_of_IfcPropertySetDefinitionSet.md", + "focus_step_id": 139, + "focus_ifc_type": "IfcRelDefinesByProperties", + "graph": { + "IfcPropertySetDefinitionSet": "IfcPropertySetDefinitionSet", + }, + }] + + assert Counter(result["template"] for result in results) == Counter({ + "Use_of_property_types.md": sum( + expected_property_projection_names.values() + ), + "Usage_of_IfcPropertySetDefinitionSet.md": len( + expected_definition_set_results + ), + }) + assert { + result["focus_ifc_type"] for result in property_type_results + } == {"IfcPropertySet"} + assert { + result["graph"]["PropertyType"] for result in property_type_results + } == {"IfcPropertySingleValue"} + assert Counter( + result["graph"]["PropertySetName"] + for result in property_type_results + ) == expected_property_projection_names + assert definition_set_results == expected_definition_set_results def test_ifc2x3_curve_style_statistics(): diff --git a/backend/apps/ifc_validation/tests_statistics_query.py b/backend/apps/ifc_validation/tests_statistics_query.py index d09a4567..043e3ee2 100644 --- a/backend/apps/ifc_validation/tests_statistics_query.py +++ b/backend/apps/ifc_validation/tests_statistics_query.py @@ -1,4 +1,5 @@ import gzip +from collections import Counter from decimal import Decimal from io import StringIO from pathlib import Path @@ -46,10 +47,26 @@ ModelInstance, PsetCountHistogram, TemplateStatistic, + UserAdditionalInfo, ValidationRequest, ) +COLUMN_PROPERTY_PROJECTION_NAMES = Counter({ + "Pset_SpaceCommon": 8, + "Pset_BuildingStoreyCommon": 6, + "Pset_ColumnCommon": 4, + "Pset_SiteCommon": 1, + "Pset_EnvironmentalImpactIndicators": 1, + "Pset_ReinforcementBarPitchOfColumn": 1, + "Pset_BuildingCommon": 3, + "Pset_BuildingElementProxyCommon": 2, + "Pset_BuildingSystemCommon": 1, + "PSet_1": 1, + "PSet_2": 1, +}) + + class StatisticsValueTests(SimpleTestCase): def test_celery_beat_uses_the_renamed_statistics_task_module(self): schedule = settings.CELERY_BEAT_SCHEDULE[ @@ -175,6 +192,19 @@ def test_template_graph_group_rejects_an_invalid_json_path(self): assert not form.is_valid() assert "value" in form.errors + def test_uploader_boolean_filters_parse_true_and_false(self): + for field in ("is_vendor", "is_staff"): + for value, expected in (("true", True), ("false", False)): + form = StatisticsQueryClauseForm(data={ + "operation": "filter", + "target": f"filter:{field}", + "operator": "eq", + "value": value, + }) + + assert form.is_valid(), form.errors + assert form.cleaned_data["typed_value"] is expected + def test_dimension_values_are_not_treated_as_numbers(self): assert format_statistics_value("IFC4") == "IFC4" assert format_statistics_value("IfcWall") == "IfcWall" @@ -240,7 +270,9 @@ def test_template_statistics_are_extracted_in_a_subprocess(self): ("Use_of_property_types.md",), ) - assert len(results) == 17 + assert Counter( + result["graph"]["PropertySetName"] for result in results + ) == COLUMN_PROPERTY_PROJECTION_NAMES assert {result["template"] for result in results} == { "Use_of_property_types.md", } @@ -261,7 +293,9 @@ def test_all_statistics_are_extracted_from_a_retained_gzip_file(self): assert entities["schema_identifier"] == "IFC4X3_ADD2" assert psets["schema_identifier"] == "IFC4X3_ADD2" - assert len(templates) == 17 + assert Counter( + result["graph"]["PropertySetName"] for result in templates + ) == COLUMN_PROPERTY_PROJECTION_NAMES def test_pset_definition_resources_cover_schema_addenda(self): assert pset_resource_schema("IFC2X3_TC1") == "IFC2X3" @@ -802,6 +836,54 @@ def test_scheduler_uses_completion_markers_instead_of_data_rows(self): assert schedule_model_statistic_tasks.run(batch_size=10) == 0 task_group.assert_not_called() + def test_template_task_stores_every_extracted_graph_projection(self): + model = Model.objects.create( + file_name="ColumnPSetsOfSets.ifc", + file="ColumnPSetsOfSets.ifc", + size=1, + schema="IFC4X3_ADD2", + uploaded_by=self.user, + ) + file_path = ( + Path(__file__).parent + / "checks" + / "statistics" + / "tests" + / "ColumnPSetsOfSets.ifc" + ) + task_module = "apps.ifc_validation.tasks.statistics_tasks" + + with patch( + f"{task_module}.get_absolute_file_path", + return_value=str(file_path), + ): + assert populate_template_statistics.run( + model.pk, + ("Use_of_property_types.md",), + ) == sum(COLUMN_PROPERTY_PROJECTION_NAMES.values()) + + projections = model.template_statistics.filter(graph__isnull=False) + assert Counter( + projection.graph["PropertySetName"] + for projection in projections + ) == COLUMN_PROPERTY_PROJECTION_NAMES + + column_common = projections.filter( + focus_instance__stepfile_id=97, + ) + assert column_common.count() == 3 + assert all( + projection.graph == { + "PropertySetName": "Pset_ColumnCommon", + "PropertyType": "IfcPropertySingleValue", + } + for projection in column_common + ) + assert model.template_statistics.filter( + template_name="Use_of_property_types.md", + graph__isnull=True, + ).exists() + def test_template_task_replaces_selected_templates_and_marks_each_one(self): model = Model.objects.create( file_name="templates.ifc", @@ -946,6 +1028,106 @@ def test_average_entity_counts_and_number_of_models(self): assert average.rows[0] == ["IFC4", "IfcWall", 20] assert model_count.rows == [["IFC4", "IfcWall", 2]] + def test_uploader_vendor_and_staff_filters(self): + verified_vendor = get_user_model().objects.create_user( + username="verified-vendor", + ) + non_vendor = get_user_model().objects.create_user(username="non-vendor") + no_additional_info = get_user_model().objects.create_user( + username="no-additional-info", + ) + UserAdditionalInfo.objects.bulk_create([ + UserAdditionalInfo( + user=self.user, + is_vendor=False, + is_vendor_self_declared=True, + created_by=self.user, + ), + UserAdditionalInfo( + user=verified_vendor, + is_vendor=True, + is_vendor_self_declared=False, + created_by=self.user, + ), + UserAdditionalInfo( + user=non_vendor, + is_vendor=False, + is_vendor_self_declared=False, + created_by=self.user, + ), + ]) + + entity_index = EntityCountHistogram.index_from_string("IFC4", "IfcWall") + extra_models = [] + for uploader in (verified_vendor, non_vendor, no_additional_info): + model = Model.objects.create( + file_name=f"{uploader.username}.ifc", + file=f"{uploader.username}.ifc", + size=1, + schema="IFC4", + uploaded_by=uploader, + ) + extra_models.append(model) + EntityCountHistogram.objects.bulk_create([ + EntityCountHistogram( + model=model, + entity_index=entity_index, + is_supertype=False, + count=1, + ), + EntityCountHistogram.completion_marker(model), + ]) + + def filtered_model_ids(field, value): + result = self.execute( + group_by="model", + limit=100, + filters=[self.clause(field, "eq", str(value).lower(), value)], + ) + return {row[0] for row in result.rows} + + verified_model, non_vendor_model, no_info_model = extra_models + assert filtered_model_ids("is_vendor", True) == { + self.first.pk, + self.second.pk, + verified_model.pk, + } + assert filtered_model_ids("is_vendor", False) == { + non_vendor_model.pk, + no_info_model.pk, + } + assert filtered_model_ids("is_staff", True) == { + self.first.pk, + self.second.pk, + } + assert filtered_model_ids("is_staff", False) == { + verified_model.pk, + non_vendor_model.pk, + no_info_model.pk, + } + for source in ("pset", "template"): + result = self.execute( + source=source, + group_by="model", + limit=100, + filters=[self.clause("is_vendor", "eq", "true", True)], + ) + assert {row[0] for row in result.rows} == { + self.first.pk, + self.second.pk, + } + + vendor_average = self.execute( + expression="count / computed_models", + filters=[ + self.clause("schema", "eq", "IFC4"), + self.clause("entity", "eq", "IfcWall"), + self.clause("entity_kind", "eq", "concrete", False), + self.clause("is_vendor", "eq", "true", True), + ], + ) + self.assertAlmostEqual(vendor_average.rows[0][-1], 41 / 3) + def test_explicit_division_by_computed_models(self): result = self.execute( expression="count / computed_models", @@ -1340,6 +1522,10 @@ def test_source_controls_available_filter_and_group_choices(self): assert "filter:pset_name" not in entity_filters assert "filter:pset_name" in pset_filters assert "filter:count" not in template_filters + for uploader_filter in ("filter:is_vendor", "filter:is_staff"): + assert uploader_filter in entity_filters + assert uploader_filter in pset_filters + assert uploader_filter in template_filters assert "group:template" in template_groups assert "group:authoring_tool" in template_groups assert "group:graph_value" in template_groups From 038d34bf7980b19a4db8883aafb0ba27c1c1be80 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Fri, 28 Aug 2026 14:45:45 +0200 Subject: [PATCH 2/8] Make all Model and ModelInstance admin fields readonly to prevent dropdown of death --- backend/apps/ifc_validation/admin.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/backend/apps/ifc_validation/admin.py b/backend/apps/ifc_validation/admin.py index 084d334c..fffa621d 100644 --- a/backend/apps/ifc_validation/admin.py +++ b/backend/apps/ifc_validation/admin.py @@ -86,7 +86,7 @@ class ValidationRequestAdmin(BaseAdmin, NonAdminAddable): ] list_display = ["id", "public_id", "file_name", "file_size_text", "authoring_tool_link", "model_link", "status", "progress", "queue_time_text", "duration_text", "is_vendor", "is_vendor_self_declared", "is_deleted", "channel_text", "created", "created_by_link", "updated", "updated_by"] - readonly_fields = ["id", "public_id", "model", "deleted", "file_name", "file", "file_size_text", "duration_text", "started", "completed", "channel", "created", "created_by", "updated", "updated_by", "file_removed"] + readonly_fields = ["id", "public_id", "model", "deleted", "file_name", "file", "file_size_text", "duration_text", "started", "completed", "channel", "created", "created_by", "updated", "updated_by", "file_removed"] date_hierarchy = "created" list_filter = [ @@ -471,7 +471,7 @@ def queue_time_text(self, obj): class ValidationOutcomeAdmin(BaseAdmin, NonAdminAddable): list_display = ["id", "public_id", "model_text", "instance_id", "type_text", "feature", "feature_version", "outcome_code", "severity", "is_whitelisted", "expected", "observed", "created", "updated"] - readonly_fields = ["id", "public_id", "created", "updated"] + readonly_fields = ["id", "public_id", "instance", "created", "updated"] list_filter = ['validation_task__type', 'severity_in_db', 'outcome_code', ('created', AdvancedDateFilter)] search_fields = ('validation_task__request__file_name', 'feature', 'feature_version', 'outcome_code', 'severity_in_db', 'expected', 'observed') @@ -665,6 +665,7 @@ def authoring_tool_link(self, obj): class ModelInstanceAdmin(BaseAdmin, NonAdminAddable): list_display = ["id", "public_id", "model", "stepfile_id", "ifc_type", "created", "updated"] + readonly_fields = ["model"] search_fields = ('stepfile_id', 'model__file_name', 'ifc_type') list_filter = ["ifc_type", "model_id", ('created', AdvancedDateFilter)] @@ -673,7 +674,7 @@ class ModelInstanceAdmin(BaseAdmin, NonAdminAddable): class EntityCountHistogramAdmin(admin.ModelAdmin): - readonly_fields = ["entity_name"] + readonly_fields = ["model", "entity_name"] @admin.display(description="Entity name") def entity_name(self, obj): @@ -681,13 +682,17 @@ def entity_name(self, obj): class PsetCountHistogramAdmin(admin.ModelAdmin): - readonly_fields = ["entity_name"] + readonly_fields = ["model", "entity_name"] @admin.display(description="Entity name") def entity_name(self, obj): return obj.entity_name +class TemplateStatisticAdmin(admin.ModelAdmin): + readonly_fields = ["model", "focus_instance"] + + class CompanyAdmin(BaseAdmin): fieldsets = [ @@ -1054,7 +1059,7 @@ class WhiteListTestForm(forms.Form): admin.site.register(Model, ModelAdmin) admin.site.register(EntityCountHistogram, EntityCountHistogramAdmin) admin.site.register(PsetCountHistogram, PsetCountHistogramAdmin) -admin.site.register(TemplateStatistic) +admin.site.register(TemplateStatistic, TemplateStatisticAdmin) admin.site.register(ModelInstance, ModelInstanceAdmin) admin.site.register(Company, CompanyAdmin) admin.site.register(AuthoringTool, AuthoringToolAdmin) From d4ca018a2cab32bc1d7277398f7127a7f0d99c79 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Sun, 30 Aug 2026 11:17:04 +0200 Subject: [PATCH 3/8] Default MODEL_STATISTIC_BATCH_SIZE to 2 --- backend/core/settings.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/backend/core/settings.py b/backend/core/settings.py index 29d86002..7b6a1761 100644 --- a/backend/core/settings.py +++ b/backend/core/settings.py @@ -374,19 +374,19 @@ raise ImproperlyConfigured(msg.format(os.path.dirname(CELERY_BEAT_SCHEDULE_FILENAME), err)) ARCHIVE_FILES_LOOKBACK_PERIOD = os.environ.get("ARCHIVE_FILES_LOOKBACK_PERIOD", 90) -REMOVE_FILES_LOOKBACK_PERIOD = os.environ.get("REMOVE_FILES_LOOKBACK_PERIOD", 180) -MODEL_STATISTIC_BATCH_SIZE = int(os.environ.get("MODEL_STATISTIC_BATCH_SIZE", 100)) -MODEL_STATISTIC_CPU_THRESHOLD = float(os.environ.get("MODEL_STATISTIC_CPU_THRESHOLD", 50)) -CELERY_BEAT_SCHEDULE = { - 'schedule-model-statistic-tasks-every-15min': { - 'task': 'apps.ifc_validation.tasks.statistics_tasks.schedule_model_statistic_tasks', - 'schedule': crontab(minute='5,20,35,50'), - 'kwargs': { - 'batch_size': MODEL_STATISTIC_BATCH_SIZE, - 'cpu_threshold': MODEL_STATISTIC_CPU_THRESHOLD, - }, - }, - 'archive-files-90days-every-15min': { +REMOVE_FILES_LOOKBACK_PERIOD = os.environ.get("REMOVE_FILES_LOOKBACK_PERIOD", 180) +MODEL_STATISTIC_BATCH_SIZE = int(os.environ.get("MODEL_STATISTIC_BATCH_SIZE", 2)) +MODEL_STATISTIC_CPU_THRESHOLD = float(os.environ.get("MODEL_STATISTIC_CPU_THRESHOLD", 50)) +CELERY_BEAT_SCHEDULE = { + 'schedule-model-statistic-tasks-every-15min': { + 'task': 'apps.ifc_validation.tasks.statistics_tasks.schedule_model_statistic_tasks', + 'schedule': crontab(minute='5,20,35,50'), + 'kwargs': { + 'batch_size': MODEL_STATISTIC_BATCH_SIZE, + 'cpu_threshold': MODEL_STATISTIC_CPU_THRESHOLD, + }, + }, + 'archive-files-90days-every-15min': { 'task': 'apps.ifc_validation.tasks.file_retention_tasks.apply_file_retention', 'schedule': crontab(minute='15,30,45'), # runs every 15 min, except at the hour 'kwargs': { 'days': ARCHIVE_FILES_LOOKBACK_PERIOD, 'dry_run': False, 'action': 'archive' }, From b54d25d6708104e73a84d82bc8792c160c228023 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Sun, 30 Aug 2026 11:23:33 +0200 Subject: [PATCH 4/8] Set PYTHON_MVDXML_REF back to master now PR is merged --- backend/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/Makefile b/backend/Makefile index 5e3ea24c..4c93db89 100644 --- a/backend/Makefile +++ b/backend/Makefile @@ -4,7 +4,7 @@ VIRTUAL_ENV = .dev/venv PYTHON = $(VIRTUAL_ENV)/bin/python PIP = $(VIRTUAL_ENV)/bin/pip IFCOPENSHELL_SITE_PACKAGES = $(VIRTUAL_ENV)/lib/python3.11/site-packages -PYTHON_MVDXML_REF = support-4.x-graphviz-format-2 +PYTHON_MVDXML_REF = master none: @echo "MAKE: Enter at least one target (venv, install, install-dev, start-backend, start-worker, clean)" From 4b970b59c4223c3935ecc3fa058c2b44bac7e83a Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Sun, 30 Aug 2026 12:11:06 +0200 Subject: [PATCH 5/8] Add test that takes statistics ratio of Proxy / Element --- .../tests/test_statistics_tasks.py | 155 ++++++++++++++++++ 1 file changed, 155 insertions(+) create mode 100644 backend/apps/ifc_validation/tests/test_statistics_tasks.py diff --git a/backend/apps/ifc_validation/tests/test_statistics_tasks.py b/backend/apps/ifc_validation/tests/test_statistics_tasks.py new file mode 100644 index 00000000..23b47f26 --- /dev/null +++ b/backend/apps/ifc_validation/tests/test_statistics_tasks.py @@ -0,0 +1,155 @@ +from pathlib import Path +from tempfile import TemporaryDirectory +from unittest.mock import patch + +from django.contrib.auth.models import User +from django.test import TransactionTestCase, override_settings +import ifcopenshell + +from apps.ifc_validation_models.models import ( + EntityCountHistogram, + ValidationRequest, + set_user_context, +) + +from ..statistics_query import StatisticsQueryBuilder +from ..statistics_query_concepts import ( + QueryFilter, + StatisticsAnnotation, + StatisticsExpression, + StatisticsQuery, +) +from ..tasks.statistics_tasks import populate_entity_count_histogram +from ..tasks.utils import get_absolute_file_path, get_or_create_ifc_model + + +class PopulateEntityCountHistogramTestCase(TransactionTestCase): + + @staticmethod + def set_user_context(): + user, _ = User.objects.get_or_create( + id=1, + defaults={ + "username": "SYSTEM", + "is_active": True, + }, + ) + set_user_context(user) + + def test_populate_entity_count_histogram(self): + self.set_user_context() + cases = ((0, 1), (1, 0), (1, 1)) + + with TemporaryDirectory() as media_root, override_settings( + MEDIA_ROOT=media_root, + ), patch( + "apps.ifc_validation.tasks.utils.MEDIA_ROOT", + media_root, + ): + get_absolute_file_path.cache_clear() + try: + for wall_count, proxy_count in cases: + ifc_file = ifcopenshell.file(schema="IFC4") + for _ in range(wall_count): + ifc_file.create_entity("IfcWall") + for _ in range(proxy_count): + ifc_file.create_entity("IfcBuildingElementProxy") + + file_name = f"walls-{wall_count}-proxies-{proxy_count}.ifc" + file_path = Path(media_root) / file_name + ifc_file.write(str(file_path)) + + request = ValidationRequest.objects.create( + file_name=file_name, + file=file_name, + size=file_path.stat().st_size, + ) + request.mark_as_initiated() + model = get_or_create_ifc_model(request.id) + model.schema = ifc_file.schema_identifier + model.save(update_fields=("schema",)) + + populated_count = populate_entity_count_histogram(model.id) + entries = EntityCountHistogram.objects.filter(model=model) + + self.assertEqual( + populated_count, + entries.filter(count__gt=0).count(), + ) + self.assertEqual(entries.filter(count=0).count(), 1) + + concrete_counts = { + entry.entity_name: entry.count + for entry in entries.filter( + count__gt=0, + is_supertype=False, + ) + } + expected_concrete_counts = { + entity_name: count + for entity_name, count in ( + ("IfcWall", wall_count), + ("IfcBuildingElementProxy", proxy_count), + ) + if count + } + self.assertEqual( + concrete_counts, + expected_concrete_counts, + ) + + for entity_name in ("IfcBuildingElement", "IfcElement"): + inherited_entry = entries.get( + entity_index=EntityCountHistogram.index_from_string( + model.schema, + entity_name, + ), + is_supertype=True, + ) + self.assertEqual( + inherited_entry.count, + wall_count + proxy_count, + ) + + query = StatisticsQuery( + source="entity", + groups=(), + expression=StatisticsExpression( + operand_a="proxy_count", + operator="divide", + operand_b="building_element_count", + ), + filters=( + QueryFilter("model", "eq", model.id), + ), + annotations=( + StatisticsAnnotation( + "proxy_count", + filters=(QueryFilter( + "entity", + "eq", + "IfcBuildingElementProxy", + ),), + ), + StatisticsAnnotation( + "building_element_count", + filters=(QueryFilter( + "entity", + "eq", + "IfcBuildingElement", + ),), + ), + ), + ) + ratio_result = StatisticsQueryBuilder(query).execute() + total = wall_count + proxy_count + self.assertEqual( + ratio_result.columns, + ["proxy_count / building_element_count"], + ) + self.assertAlmostEqual( + ratio_result.rows[0][0], + proxy_count / total, + ) + finally: + get_absolute_file_path.cache_clear() From be2bd3dd9fa47abeef71bcbb6700dca7f4763b0f Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Sun, 30 Aug 2026 13:33:51 +0200 Subject: [PATCH 6/8] Remove statistics task scheduling for the time being --- backend/core/settings.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/backend/core/settings.py b/backend/core/settings.py index 7b6a1761..caab027c 100644 --- a/backend/core/settings.py +++ b/backend/core/settings.py @@ -377,16 +377,8 @@ REMOVE_FILES_LOOKBACK_PERIOD = os.environ.get("REMOVE_FILES_LOOKBACK_PERIOD", 180) MODEL_STATISTIC_BATCH_SIZE = int(os.environ.get("MODEL_STATISTIC_BATCH_SIZE", 2)) MODEL_STATISTIC_CPU_THRESHOLD = float(os.environ.get("MODEL_STATISTIC_CPU_THRESHOLD", 50)) -CELERY_BEAT_SCHEDULE = { - 'schedule-model-statistic-tasks-every-15min': { - 'task': 'apps.ifc_validation.tasks.statistics_tasks.schedule_model_statistic_tasks', - 'schedule': crontab(minute='5,20,35,50'), - 'kwargs': { - 'batch_size': MODEL_STATISTIC_BATCH_SIZE, - 'cpu_threshold': MODEL_STATISTIC_CPU_THRESHOLD, - }, - }, - 'archive-files-90days-every-15min': { +CELERY_BEAT_SCHEDULE = { + 'archive-files-90days-every-15min': { 'task': 'apps.ifc_validation.tasks.file_retention_tasks.apply_file_retention', 'schedule': crontab(minute='15,30,45'), # runs every 15 min, except at the hour 'kwargs': { 'days': ARCHIVE_FILES_LOOKBACK_PERIOD, 'dry_run': False, 'action': 'archive' }, From b25f66a0c275650e6026c3a94c2df799a77030c3 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Sun, 30 Aug 2026 13:34:09 +0200 Subject: [PATCH 7/8] More ai-directed work on statistics --- .../apps/ifc_validation/statistics_query.py | 484 +++++++++++++++--- .../statistics_query_concepts.py | 71 ++- .../statistics_query_examples.py | 53 +- .../templates/admin/model_statistics.html | 104 +++- backend/apps/ifc_validation/test_settings.py | 3 +- .../ifc_validation/tests_statistics_query.py | 328 +++++++++++- 6 files changed, 915 insertions(+), 128 deletions(-) diff --git a/backend/apps/ifc_validation/statistics_query.py b/backend/apps/ifc_validation/statistics_query.py index 3478e5c3..45776a18 100644 --- a/backend/apps/ifc_validation/statistics_query.py +++ b/backend/apps/ifc_validation/statistics_query.py @@ -47,6 +47,7 @@ SOURCE, SOURCES, QueryFilter, + StatisticsAnnotation, StatisticsExpression, StatisticsQuery, choices, @@ -64,10 +65,18 @@ class StatisticsQueryClauseForm(forms.Form): target = forms.ChoiceField(choices=[ *((f"filter:{concept.name}", concept.label) for concept in CONCEPTS if "filter" in concept.acts_in), *((f"group:{concept.name}", concept.label) for concept in CONCEPTS if "group" in concept.acts_in), + ("annotate:none", "No additional condition"), + *((f"annotate:{concept.name}", f"Where {concept.label}") + for concept in CONCEPTS if "filter" in concept.acts_in), *((f"order:{ordering.name}", ordering.label) for ordering in ORDERINGS), ], required=False) operator = forms.ChoiceField(choices=choices(QUERY_OPERATORS), required=False) value = forms.CharField(max_length=1024, required=False) + annotation_name = forms.CharField( + max_length=64, + required=False, + widget=forms.TextInput(attrs={"placeholder": "Name"}), + ) expression_function = forms.ChoiceField( choices=choices(FUNCTIONS), required=False, @@ -89,6 +98,35 @@ class StatisticsQueryClauseForm(forms.Form): widget=forms.Select(attrs={"aria-label": "Operand B"}), ) + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + annotation_names = dict.fromkeys( + str(value).strip() + for key, value in self.data.items() + if key.endswith("-annotation_name") and str(value).strip() + ) + for field_name, placeholder in (("operand_a", "๐‘Ž"), ("operand_b", "๐‘")): + self.fields[field_name].choices = [ + ("", placeholder), + *choices(OPERANDS), + *((name, name) for name in annotation_names), + ] + + def clean_expression(self, cleaned): + operand_a = cleaned.get("operand_a") + expression_operator = cleaned.get("expression_operator") + operand_b = cleaned.get("operand_b") + if not operand_a: + self.add_error("operand_a", "Select operand A.") + if expression_operator and not operand_b: + self.add_error("operand_b", "Select operand B.") + if operand_b and not expression_operator: + self.add_error("expression_operator", "Select an operator.") + return StatisticsExpression( + cleaned.get("expression_function"), operand_a, + expression_operator, operand_b, + ) + def clean(self): cleaned = super().clean() if cleaned.get("DELETE"): @@ -115,28 +153,43 @@ def clean(self): cleaned["resolved_value"] = limit return cleaned + clause_expression = None + if operation in {"annotate", "expression"}: + clause_expression = self.clean_expression(cleaned) if operation == "expression": - operand_a = cleaned.get("operand_a") - expression_operator = cleaned.get("expression_operator") - operand_b = cleaned.get("operand_b") - if not operand_a: - self.add_error("operand_a", "Select operand A.") - if expression_operator and not operand_b: - self.add_error("operand_b", "Select operand B.") - if operand_b and not expression_operator: - self.add_error("expression_operator", "Select an operator.") - cleaned["resolved_value"] = StatisticsExpression( - cleaned.get("expression_function"), operand_a, - expression_operator, operand_b, - ) + cleaned["resolved_value"] = clause_expression return cleaned + annotation_name = cleaned.get("annotation_name", "").strip() + if operation == "annotate": + if not re.fullmatch(r"[A-Za-z_][A-Za-z0-9_]*", annotation_name): + self.add_error( + "annotation_name", + "Enter a name using letters, numbers, and underscores.", + ) + elif annotation_name in OPERAND: + self.add_error( + "annotation_name", + "Annotation names cannot replace built-in operands.", + ) + if clause_expression.function == "average": + self.add_error( + "expression_function", + "AVG is available only in the final Expression clause.", + ) + expected_prefix = f"{operation}:" if not target or not target.startswith(expected_prefix): self.add_error("target", "Select a value for this operation.") return cleaned resolved_value = target.removeprefix(expected_prefix) cleaned["resolved_value"] = resolved_value + if operation == "annotate" and resolved_value == "none": + cleaned["resolved_value"] = StatisticsAnnotation( + annotation_name, + clause_expression, + ) + return cleaned if operation == "group" and resolved_value == "graph_value": if not value: self.add_error("value", "Enter a JSON graph path.") @@ -148,7 +201,7 @@ def clean(self): else: cleaned["resolved_value"] = f"graph_value:{value}" return cleaned - if operation != "filter": + if operation not in {"filter", "annotate"}: return cleaned field = resolved_value @@ -170,6 +223,12 @@ def clean(self): else: cleaned["value"] = value cleaned["typed_value"] = typed_value + if operation == "annotate": + cleaned["resolved_value"] = StatisticsAnnotation( + annotation_name, + clause_expression, + (QueryFilter(field, operator, typed_value),), + ) return cleaned StatisticsQueryClauseFormSet = forms.formset_factory( @@ -210,14 +269,37 @@ def query_form_clauses(query): if graph_path: clause["value"] = graph_path clauses.append(clause) + for annotation in query.annotations: + if len(annotation.filters) > 1: + raise ValueError( + "The query form supports one condition per Annotate clause.", + ) + clause = { + "operation": "annotate", + "annotation_name": annotation.name, + "target": "annotate:none", + "expression_function": annotation.expression.function, + "operand_a": annotation.expression.operand_a, + "expression_operator": annotation.expression.operator, + "operand_b": annotation.expression.operand_b, + } + if annotation.filters: + item = annotation.filters[0] + clause.update({ + "target": f"annotate:{item.concept}", + "operator": item.operator, + "value": CONCEPT[item.concept].serialize(item.value), + }) + clauses.append(clause) clauses.extend(( {"operation": "expression", "expression_function": query.expression.function, "operand_a": query.expression.operand_a, "expression_operator": query.expression.operator, "operand_b": query.expression.operand_b}, {"operation": "order", "target": f"order:{query.ordering}"}, - {"operation": "limit", "value": query.limit if query.limit is not None else "all"}, )) + if query.limit is not None: + clauses.append({"operation": "limit", "value": query.limit}) return clauses @@ -246,14 +328,20 @@ def build_statistics_specification(source, clause_formset): operation: [clause for clause in clauses if clause["operation"] == operation] for operation in OPERATION } - if not operations["group"]: - raise ValueError("The query requires at least one group clause.") if len(operations["expression"]) != 1: raise ValueError("The query requires exactly one expression clause.") for operation in ("order", "limit"): if len(operations[operation]) > 1: raise ValueError(f"The query accepts at most one {operation} clause.") + annotations = tuple( + clause["resolved_value"] + for clause in operations["annotate"] + ) + annotation_names = [annotation.name for annotation in annotations] + if len(annotation_names) != len(set(annotation_names)): + raise ValueError("Annotation names must be unique.") + return StatisticsQuery( source=source, groups=tuple( @@ -270,32 +358,64 @@ def build_statistics_specification(source, clause_formset): QueryFilter(clause["resolved_value"], clause["operator"], clause["typed_value"]) for clause in operations["filter"] ), + annotations=annotations, ) +def _example_expression_context(clause): + expression = StatisticsExpression( + clause["expression_function"], clause["operand_a"], + clause["expression_operator"], clause["operand_b"], + ) + return { + "function": FUNCTION[expression.function].label, + "function_active": bool(expression.function), + "operand_a": ( + OPERAND[expression.operand_a].label + if expression.operand_a in OPERAND else expression.operand_a + ), + "operator": EXPRESSION_OPERATOR[expression.operator].label, + "operator_active": bool(expression.operator), + "operand_b": ( + OPERAND[expression.operand_b].label + if expression.operand_b in OPERAND + else expression.operand_b or "๐‘" + ), + "operand_b_active": bool(expression.operand_b), + } + + def _example_clause_context(clause): operation = clause["operation"] if operation == "expression": - expression = StatisticsExpression( - clause["expression_function"], clause["operand_a"], - clause["expression_operator"], clause["operand_b"], - ) return { "operation": OPERATION[operation].label, - "expression": { - "function": FUNCTION[expression.function].label, - "function_active": bool(expression.function), - "operand_a": OPERAND[expression.operand_a].label, - "operator": EXPRESSION_OPERATOR[expression.operator].label, - "operator_active": bool(expression.operator), - "operand_b": OPERAND[expression.operand_b].label if expression.operand_b else "๐‘", - "operand_b_active": bool(expression.operand_b), - }, + "expression": _example_expression_context(clause), "form": clause, } if operation == "limit": return {"operation": OPERATION[operation].label, "selection": str(clause["value"]), "operator": "", "value": "", "form": clause} + if operation == "annotate": + _, target = clause["target"].split(":", 1) + condition = "" + if target != "none": + condition = " ".join(( + "where", + CONCEPT[target].label, + QUERY_OPERATOR[clause["operator"]].label, + str(clause.get("value", "")), + )) + expression_context = _example_expression_context(clause) + expression_context.update({ + "annotation_name": clause["annotation_name"], + "condition": condition, + }) + return { + "operation": OPERATION[operation].label, + "expression": expression_context, + "form": clause, + } _, target = clause["target"].split(":", 1) selection = ORDERING[target].label if operation == "order" else CONCEPT[target].label return {"operation": OPERATION[operation].label, "selection": selection, @@ -349,6 +469,17 @@ def statistics_query_ui_context(): ] for source in SOURCE }, + "annotate": { + source: [ + {"value": "annotate:none", "label": "No additional condition"}, + *( + {"value": f"annotate:{concept.name}", + "label": f"Where {concept.label}"} + for concept in CONCEPTS if concept.supports("filter", source) + ), + ] + for source in SOURCE + }, "expression": [], "order": [ {"value": f"order:{value}", "label": label} @@ -357,12 +488,21 @@ def statistics_query_ui_context(): "limit": [], }, "filter_operator_choices": { - f"filter:{field}": [ + f"{operation}:{field}": [ {"value": operator, "label": QUERY_OPERATOR[operator].label} for operator in concept.operators ] + for operation in ("filter", "annotate") for field, concept in CONCEPT.items() if "filter" in concept.acts_in }, + "statistics_operand_choices": [ + {"value": value, "label": label} + for value, label in choices(OPERANDS) + ], + "statistics_function_choices": [ + {"value": value, "label": label} + for value, label in choices(FUNCTIONS) + ], "filter_suggestions": { "models": [ (str(model.pk), f"#{model.pk} - {model.file_name}") @@ -419,7 +559,9 @@ def __init__(self, specification): self.source = SOURCE[specification.source] self.filters = specification.filters self.groups = specification.groups + self.annotations = specification.annotations self.schema = self.resolve_schema() + self.validate_annotations() def resolve_schema(self): model_ids = { @@ -485,6 +627,62 @@ def apply_filter(self, query, clause): query, concept.lookup, clause.operator, clause.value, ) + def filter_condition(self, clause): + concept = CONCEPT[clause.concept] + if not concept.supports("filter", self.source.name): + raise ValueError( + f"Filter {concept.name!r} is not available for {self.source.name!r}.", + ) + if clause.operator not in concept.operators: + raise ValueError( + f"Operator {clause.operator!r} is not available for {concept.name!r}.", + ) + if concept.name == "entity": + if not self.schema: + raise ValueError( + "Entity annotation filters require one model or one exact schema filter.", + ) + if clause.operator in {"subtype_of", "not_subtype_of"}: + values = self.subtype_indices(self.schema, clause.value) + if self.source.name == "template": + values = [ + EntityCountHistogram.string_from_index(self.schema, index) + for index in values + ] + lookup = "focus_instance__ifc_type__in" + else: + lookup = "entity_index__in" + condition = Q(**{lookup: values}) + return ~condition if clause.operator == "not_subtype_of" else condition + value = ( + clause.value + if self.source.name == "template" + else EntityCountHistogram.index_from_string(self.schema, clause.value) + ) + lookup = ( + "focus_instance__ifc_type" + if self.source.name == "template" else "entity_index" + ) + condition = Q(**{lookup: value}) + return ~condition if clause.operator == "ne" else condition + if concept.name == "is_vendor": + vendor = ( + Q(model__uploaded_by__useradditionalinfo__is_vendor=True) + | Q(model__uploaded_by__useradditionalinfo__is_vendor_self_declared=True) + ) + matches = vendor if clause.value else ~vendor + return ~matches if clause.operator == "ne" else matches + + operation = QUERY_OPERATOR[clause.operator] + if operation.special: + raise ValueError( + f"Operator {clause.operator!r} requires an entity concept.", + ) + condition = Q(**{ + f"{concept.lookup}{operation.suffix}": clause.value, + }) + return ~condition if operation.negated else condition + @classmethod def apply_vendor_filter(cls, query, operator, value, model_prefix): vendor_status = Case( @@ -613,18 +811,12 @@ def grouped_queryset(self, base): raise ValueError( "Proxy grouping requires entity counts and one schema or model.", ) - proxy_indices = set( - self.subtype_indices(self.schema, "IfcBuildingElementProxy"), - ) - proxy_indices.add( - EntityCountHistogram.index_from_string( - self.schema, - "IfcBuildingElementProxy", - ) - ) base = base.annotate( proxy_group=Case( - When(entity_index__in=proxy_indices, then=Value("Proxy")), + When( + entity_index__in=self.proxy_group_indices(), + then=Value("Proxy"), + ), default=Value("Other element subtypes"), output_field=CharField(), ) @@ -643,6 +835,111 @@ def grouped_queryset(self, base): def count_expression(self): return self.source.count_expression() + def proxy_group_indices(self): + if self.source.name != "entity" or not self.schema: + raise ValueError( + "Proxy grouping requires entity counts and one schema or model.", + ) + return ( + *self.subtype_indices(self.schema, "IfcBuildingElementProxy"), + EntityCountHistogram.index_from_string( + self.schema, + "IfcBuildingElementProxy", + ), + ) + + def validate_annotations(self): + names = [annotation.name for annotation in self.annotations] + if len(names) != len(set(names)): + raise ValueError("Annotation names must be unique.") + reserved_names = { + *OPERAND, + "group_count", + "source_count", + "source_models", + "statistics_scalar", + "value", + *( + name + for field in self.source.model._meta.get_fields() + for name in (field.name, getattr(field, "attname", field.name)) + ), + } + available_names = set(StatisticsExpression.NAMES) + for annotation in self.annotations: + if ( + not re.fullmatch(r"[A-Za-z_][A-Za-z0-9_]*", annotation.name) + or annotation.name in reserved_names + ): + raise ValueError(f"Unsupported annotation name {annotation.name!r}.") + annotation.expression.validate() + if annotation.expression.is_average: + raise ValueError( + "AVG is available only in the final Expression clause.", + ) + self.validate_expression_names( + annotation.expression, + available_names, + f"annotation {annotation.name!r}", + ) + for clause in annotation.filters: + self.filter_condition(clause) + available_names.add(annotation.name) + self.spec.expression.validate() + self.validate_expression_names( + self.spec.expression, + available_names, + "final expression", + ) + + def annotation_condition(self, annotation): + condition = None + for clause in annotation.filters: + clause_condition = self.filter_condition(clause) + condition = ( + clause_condition + if condition is None else condition & clause_condition + ) + return condition + + @staticmethod + def validate_expression_names(expression, available_names, label): + unknown_names = expression.names - set(available_names) + if unknown_names: + raise ValueError( + f"Unknown operand(s) in {label}: " + + ", ".join(sorted(unknown_names)), + ) + + def formula_names(self, expression): + names = set(expression.names) + for annotation in self.annotations: + names.update(annotation.expression.names) + return names + + def apply_annotations(self, query, computed_models, total_count): + aliases = {} + for annotation in self.annotations: + condition = self.annotation_condition(annotation) + count = self.source.count_expression(condition) + models = Count("model_id", distinct=True, filter=condition) + values = { + "count": Cast(count, FloatField()), + "models": Cast(models, FloatField()), + "computed_models": Value(float(computed_models or 1)), + "total_count": Value(float(total_count or 1)), + **aliases, + } + if annotation.expression.source == "count": + value = count + elif annotation.expression.source == "models": + value = models + else: + value = annotation.expression.compile(values) + query = query.annotate(**{annotation.name: value}) + aliases[annotation.name] = Cast(F(annotation.name), FloatField()) + return query, aliases + def display_key(self, fields, values): displayed = list(values) schema = ( @@ -685,11 +982,31 @@ def execute(self): expression, ) - total_count = base.aggregate(total=count_expression)["total"] or 0 - computed_models = self.computed_model_count() - query = grouped_base.values(*fields).annotate( - source_count=count_expression, - source_models=Count("model_id", distinct=True), + formula_names = self.formula_names(expression) + total_count = ( + base.aggregate(total=count_expression)["total"] or 0 + if "total_count" in formula_names else None + ) + computed_models = ( + self.computed_model_count() + if "computed_models" in formula_names else None + ) + query_fields = list(fields) + if not query_fields: + grouped_base = grouped_base.annotate(statistics_scalar=Value(1)) + query_fields.append("statistics_scalar") + query = grouped_base.values(*query_fields) + source_values = {} + if "count" in expression.names: + source_values["source_count"] = count_expression + if expression.source == "models": + source_values["source_models"] = Count("model_id", distinct=True) + if source_values: + query = query.annotate(**source_values) + query, annotation_values = self.apply_annotations( + query, + computed_models, + total_count, ) if expression.source == "count": query = query.annotate(value=F("source_count")) @@ -701,14 +1018,20 @@ def execute(self): "models": Cast(F("source_models"), FloatField()), "computed_models": Value(float(computed_models or 1)), "total_count": Value(float(total_count or 1)), + **annotation_values, } - query = query.annotate(value=expression.compile(expression_values)) + query = query.annotate( + value=expression.compile(expression_values), + ) query = query.order_by("-value" if descending else "value") if limit is not None: query = query[:limit] - raw_rows = list(query.values_list(*fields, "value")) - rows = [self.display_key(fields, row[:-1]) + [row[-1]] for row in raw_rows] + raw_rows = list(query.values_list(*query_fields, "value")) + rows = [ + self.display_key(fields, row[:len(fields)]) + [row[-1]] + for row in raw_rows + ] return StatisticsQueryResult( labels + [expression.source], rows, @@ -716,31 +1039,68 @@ def execute(self): ) def average_expression_result(self, base, fields, labels, count_expression, expression): - per_model = ( - base.values("model_id", *fields) - .annotate(group_count=count_expression) - .order_by() + denominator = self.computed_model_count() + total_count = ( + base.aggregate(total=count_expression)["total"] or 0 + if "total_count" in self.formula_names(expression) else None + ) + needs_group_count = bool( + expression.names & {"count", "model_total_count"} + ) or not self.annotations + per_model = base.values("model_id", *fields) + if needs_group_count: + per_model = per_model.annotate(group_count=count_expression) + per_model, _ = self.apply_annotations( + per_model, + denominator, + total_count, ) - records = list(per_model.values_list("model_id", *fields, "group_count")) + per_model = per_model.order_by() + annotation_names = [annotation.name for annotation in self.annotations] + value_fields = [ + *(["group_count"] if needs_group_count else []), + *annotation_names, + ] + records = list(per_model.values_list( + "model_id", + *fields, + *value_fields, + )) totals = defaultdict(float) grouped = defaultdict(float) + grouped_annotations = { + name: defaultdict(float) for name in annotation_names + } + group_keys = [] + seen_group_keys = set() for record in records: model_id = record[0] - key = tuple(record[1:-1]) - count = record[-1] - totals[model_id] += count - grouped[(model_id, key)] += count + key = tuple(record[1:1 + len(fields)]) + group_key = (model_id, key) + if group_key not in seen_group_keys: + seen_group_keys.add(group_key) + group_keys.append(group_key) + value_offset = 1 + len(fields) + if needs_group_count: + count = record[value_offset] + totals[model_id] += count + grouped[group_key] += count + value_offset += 1 + for offset, name in enumerate(annotation_names, start=value_offset): + grouped_annotations[name][(model_id, key)] += record[offset] - denominator = self.computed_model_count() - total_count = sum(totals.values()) averages = defaultdict(float) - for (model_id, key), count in grouped.items(): + for model_id, key in group_keys: averages[key] += expression.evaluate({ - "count": count, + "count": grouped[(model_id, key)], "models": 1, "computed_models": denominator, "total_count": total_count, "model_total_count": totals[model_id], + **{ + name: values[(model_id, key)] + for name, values in grouped_annotations.items() + }, }) rows = [ self.display_key(fields, key) + [value / (denominator or 1)] diff --git a/backend/apps/ifc_validation/statistics_query_concepts.py b/backend/apps/ifc_validation/statistics_query_concepts.py index d9d4ca4d..1acd1de0 100644 --- a/backend/apps/ifc_validation/statistics_query_concepts.py +++ b/backend/apps/ifc_validation/statistics_query_concepts.py @@ -72,12 +72,20 @@ | authoring_tool | Authoring tool | group | template | โ€” | | graph_value | Template graph value | group | template | โ€” | +Annotate and Expression share the ``[function](A [operator B])`` grammar. +Annotate assigns that formula a reusable name and may add one local condition +to its source aggregates. Later annotations and the final unnamed Expression +can reuse those names, so scalar measures such as ``wall_count / door_count`` +do not require a Group by clause. + """ import operator +import re from dataclasses import dataclass from django.db.models import Count, ExpressionWrapper, FloatField, Sum, Value +from django.db.models.functions import Coalesce, NullIf from apps.ifc_validation_models.models import ( EntityCountHistogram, @@ -153,8 +161,13 @@ class StatisticsSource(NamedChoice): def queryset(self): return self.model.objects.filter(**dict(self.conditions)) - def count_expression(self): - return Sum(self.count_field) if self.count_field else Count("pk") + def count_expression(self, condition=None): + options = {"filter": condition} if condition is not None else {} + if self.count_field: + if condition is not None: + options["default"] = 0 + return Sum(self.count_field, **options) + return Count("pk", **options) @dataclass(frozen=True) @@ -175,16 +188,25 @@ class StatisticsExpression: "count", "computed_models", "total_count", "model_total_count", "1", "100", }) + @staticmethod + def is_valid_operand(value): + return ( + value in OPERAND + or bool(re.fullmatch(r"[A-Za-z_][A-Za-z0-9_]*", value or "")) + ) + def validate(self): - if self.function not in FUNCTION or self.operand_a not in OPERAND: + if self.function not in FUNCTION or not self.is_valid_operand(self.operand_a): + raise ValueError("Unsupported expression function or operand.") + if self.operand_b and not self.is_valid_operand(self.operand_b): raise ValueError("Unsupported expression function or operand.") if self.operator not in EXPRESSION_OPERATOR: raise ValueError("Unsupported expression operator.") if bool(self.operator) != bool(self.operand_b): raise ValueError("An expression operator and operand B must be used together.") if self.function == "average": - if {self.operand_a, self.operand_b} - {""} <= { - "count", "model_total_count", "1", "100", + if not ({self.operand_a, self.operand_b} - {""}) & { + "model", "computed_models", "total_count", }: return raise ValueError("Unsupported AVG expression.") @@ -197,7 +219,7 @@ def validate(self): return raise ValueError("Unsupported COUNT DISTINCT expression.") operands = {self.operand_a, self.operand_b} - {""} - if not operands <= self.NAMES or "model_total_count" in operands: + if operands & {"model", "model_total_count"}: raise ValueError("Unsupported expression operand.") @property @@ -220,7 +242,10 @@ def is_average(self): @property def names(self): self.validate() - return {name for name in (self.operand_a, self.operand_b) if name in self.NAMES} + return { + name for name in (self.operand_a, self.operand_b) + if name not in {"", "1", "100", "model"} + } def _operand(self, name, values, orm=False): if name in {"1", "100"}: @@ -232,9 +257,23 @@ def compile(self, values): self.validate() expression = self._operand(self.operand_a, values, orm=True) if self.operator: - expression = EXPRESSION_OPERATOR[self.operator].function( - expression, self._operand(self.operand_b, values, orm=True), - ) + right = self._operand(self.operand_b, values, orm=True) + if self.operator == "divide": + # Django adds scalar functions around literals to GROUP BY in an + # aggregate query. Literal operands are already known here, so + # only use SQL NULLIF for denominators resolved by the database. + denominator = right + if not isinstance(right, Value): + denominator = NullIf(right, Value(0.0)) + expression = Coalesce( + expression / denominator, + Value(0.0), + ) + else: + expression = EXPRESSION_OPERATOR[self.operator].function( + expression, + right, + ) return ExpressionWrapper(expression, output_field=FloatField()) def evaluate(self, values): @@ -250,6 +289,13 @@ def evaluate(self, values): return 0 +@dataclass(frozen=True) +class StatisticsAnnotation: + name: str + expression: StatisticsExpression = StatisticsExpression() + filters: tuple = () + + @dataclass(frozen=True) class StatisticsQuery: source: str @@ -258,6 +304,7 @@ class StatisticsQuery: ordering: str = "descending" limit: int | None = 10 filters: tuple = () + annotations: tuple = () def _index(entries): @@ -266,8 +313,8 @@ def _index(entries): OPERATIONS = ( NamedChoice("filter", "Filter"), NamedChoice("group", "Group by"), - NamedChoice("expression", "Expression"), NamedChoice("order", "Order by"), - NamedChoice("limit", "Limit"), + NamedChoice("annotate", "Annotate"), NamedChoice("expression", "Expression"), + NamedChoice("order", "Order by"), NamedChoice("limit", "Limit"), ) ORDERINGS = ( NamedChoice("descending", "Descending"), NamedChoice("ascending", "Ascending"), diff --git a/backend/apps/ifc_validation/statistics_query_examples.py b/backend/apps/ifc_validation/statistics_query_examples.py index 96fc69d9..fb0b321d 100644 --- a/backend/apps/ifc_validation/statistics_query_examples.py +++ b/backend/apps/ifc_validation/statistics_query_examples.py @@ -1,12 +1,29 @@ from apps.ifc_validation.statistics_query_concepts import ( QueryFilter, + StatisticsAnnotation, StatisticsExpression, StatisticsQuery, ) -def example(title, source, groups, filters=(), expression=StatisticsExpression(), limit=10): - return title, StatisticsQuery(source, groups, expression, limit=limit, filters=filters) +def example(title, source, groups, filters=(), expression=StatisticsExpression(), limit=10, + annotations=()): + return title, StatisticsQuery( + source, groups, expression, limit=limit, filters=filters, + annotations=annotations, + ) + + +PROXY_RATIO_ANNOTATIONS = ( + StatisticsAnnotation( + "proxy_count", + filters=(QueryFilter("entity", "eq", "IfcBuildingElementProxy"),), + ), + StatisticsAnnotation( + "building_element_count", + filters=(QueryFilter("entity", "eq", "IfcBuildingElement"),), + ), +) EXAMPLES = ( @@ -16,10 +33,10 @@ def example(title, source, groups, filters=(), expression=StatisticsExpression() example("Average top 10 element subtypes used in files of an IFC version", "entity", ("entity",), (QueryFilter("schema", "eq", "IFC4"), QueryFilter("entity", "subtype_of", "IfcElement"), QueryFilter("entity_kind", "eq", False)), StatisticsExpression("average")), - example("Number of files of an IFC version containing an entity", "entity", ("entity",), + example("Number of files of an IFC version containing an entity", "entity", (), (QueryFilter("schema", "eq", "IFC4"), QueryFilter("entity", "eq", "IfcWall"), - QueryFilter("entity_kind", "eq", False), QueryFilter("count", "gt", 0)), - StatisticsExpression("count_distinct", "model")), + QueryFilter("entity_kind", "eq", False)), + StatisticsExpression("count_distinct", "model"), limit=None), example("Top 10 property sets used in one file", "pset", ("pset_name",), (QueryFilter("model", "eq", 123), QueryFilter("pset_scope", "eq", True)), StatisticsExpression("sum")), @@ -33,28 +50,30 @@ def example(title, source, groups, filters=(), expression=StatisticsExpression() ("standardized",), (QueryFilter("schema", "eq", "IFC4"), QueryFilter("pset_scope", "eq", True)), StatisticsExpression("average", operator="divide", operand_b="model_total_count")), - example("Ratio of proxy versus other element subtypes in one file", "entity", ("proxy",), - (QueryFilter("model", "eq", 123), QueryFilter("entity", "subtype_of", "IfcElement"), - QueryFilter("entity_kind", "eq", False)), - StatisticsExpression(operator="divide", operand_b="total_count")), - example("Average proxy ratio in files of an IFC version", "entity", ("proxy",), - (QueryFilter("schema", "eq", "IFC4"), QueryFilter("entity", "subtype_of", "IfcElement"), - QueryFilter("entity_kind", "eq", False)), - StatisticsExpression("average", operator="divide", operand_b="model_total_count")), + example("Proxy ratio in one file", "entity", (), + (QueryFilter("model", "eq", 123),), + StatisticsExpression(operand_a="proxy_count", operator="divide", + operand_b="building_element_count"), + limit=None, annotations=PROXY_RATIO_ANNOTATIONS), + example("Average proxy ratio in files of an IFC version", "entity", (), + (QueryFilter("schema", "eq", "IFC4"),), + StatisticsExpression("average", operand_a="proxy_count", operator="divide", + operand_b="building_element_count"), + limit=None, annotations=PROXY_RATIO_ANNOTATIONS), example("Property type counts grouped by AuthoringTool", "template", ("authoring_tool", "graph_value:PropertyType"), (QueryFilter("template", "eq", "Use_of_property_types.md"),), - StatisticsExpression("sum"), None), + StatisticsExpression("sum"), limit=None), example("Property type counts for a single model", "template", ("graph_value:PropertyType",), (QueryFilter("model", "eq", 123), QueryFilter("template", "eq", "Use_of_property_types.md")), - StatisticsExpression("sum"), None), + StatisticsExpression("sum"), limit=None), example("Basis counts grouped by AuthoringTool", "template", ("authoring_tool", "graph_value:ParentCurve"), (QueryFilter("template", "eq", "Usage_of_transition_curves_geometry.md"),), - StatisticsExpression("sum"), None), + StatisticsExpression("sum"), limit=None), example("Basis type counts for a single model", "template", ("graph_value:ParentCurve",), (QueryFilter("model", "eq", 123), QueryFilter("template", "eq", "Usage_of_transition_curves_geometry.md")), - StatisticsExpression("sum"), None), + StatisticsExpression("sum"), limit=None), ) diff --git a/backend/apps/ifc_validation/templates/admin/model_statistics.html b/backend/apps/ifc_validation/templates/admin/model_statistics.html index 744a11ca..21d0acad 100644 --- a/backend/apps/ifc_validation/templates/admin/model_statistics.html +++ b/backend/apps/ifc_validation/templates/admin/model_statistics.html @@ -13,7 +13,9 @@

Build a transient query one clause at a time. Filter clauses are combined - with AND. Nothing is saved. + with AND. Annotate names a formula that later annotations and the final + Expression can reuse. Group by is optional for a scalar result. Nothing + is saved.

Example query patterns @@ -38,12 +40,19 @@ {{ clause.operation }} {% if clause.expression %} + {% if clause.expression.annotation_name %} + {{ clause.expression.annotation_name }} + = + {% endif %} {{ clause.expression.function }} {% if clause.expression.function_active %}({% endif %} {{ clause.expression.operand_a }} {{ clause.expression.operator }} {{ clause.expression.operand_b }} {% if clause.expression.function_active %}){% endif %} + {% if clause.expression.condition %} + {{ clause.expression.condition }} + {% endif %} {% else %} {% if clause.selection %}{{ clause.selection }}{% else %}{% endif %} @@ -87,6 +96,7 @@

Query

{{ clause_form.operator }}
{{ clause_form.value }}
+ {{ clause_form.annotation_name }} = {{ clause_form.expression_function }} ( {{ clause_form.operand_a }} @@ -148,6 +158,7 @@

Query

{{ clause_formset.empty_form.operator }}
{{ clause_formset.empty_form.value }}
+ {{ clause_formset.empty_form.annotation_name }} = {{ clause_formset.empty_form.expression_function }} ( {{ clause_formset.empty_form.operand_a }} @@ -193,6 +204,8 @@

Query

{{ clause_target_choices|json_script:"statistics-clause-targets" }} {{ filter_operator_choices|json_script:"statistics-filter-operators" }} + {{ statistics_operand_choices|json_script:"statistics-operand-choices" }} + {{ statistics_function_choices|json_script:"statistics-function-choices" }} {{ statistics_query_examples|json_script:"statistics-query-examples" }}