diff --git a/flow360/component/simulation/validation/validation_simulation_params.py b/flow360/component/simulation/validation/validation_simulation_params.py index 065d1c143..e4167491a 100644 --- a/flow360/component/simulation/validation/validation_simulation_params.py +++ b/flow360/component/simulation/validation/validation_simulation_params.py @@ -368,13 +368,16 @@ def _collect_volume_zones(params) -> list: return [] -def _collect_asset_boundary_entities(params, param_info: ParamsValidationInfo) -> list: +def _collect_asset_boundary_entities(params, param_info: ParamsValidationInfo) -> tuple[list, bool]: """Collect boundary entities that should be considered valid for BC completeness checks. This includes: - Persistent boundaries from asset cache - Farfield-related ghost boundaries, conditional on farfield method - Wind tunnel ghost surfaces (when applicable) + + Returns: + tuple: (asset_boundary_entities, has_missing_private_attributes) """ # IMPORTANT: # AssetCache.boundaries may return a direct reference into EntityInfo internal lists @@ -382,12 +385,27 @@ def _collect_asset_boundary_entities(params, param_info: ParamsValidationInfo) - # mutating entity_info and corrupting subsequent serialization/validation. asset_boundary_entities = list(params.private_attribute_asset_cache.boundaries or []) farfield_method = params.meshing.farfield_method if params.meshing else None + has_missing_private_attributes = False if not farfield_method: - return asset_boundary_entities + return asset_boundary_entities, has_missing_private_attributes + + # Check for legacy assets missing private_attributes before farfield-related processing + # This check is only relevant when we need bounding box information for farfield operations + # Only flag as legacy if ALL boundaries are missing private_attributes (not just some) + # AND the farfield method is one that performs automatic surface deletion (auto/quasi-3d modes) + # For user-defined/wind-tunnel, missing BCs are always errors since no auto-deletion occurs + if ( + asset_boundary_entities + and farfield_method in ("auto", "quasi-3d", "quasi-3d-periodic") + and all( + getattr(item, "private_attributes", None) is None for item in asset_boundary_entities + ) + ): + has_missing_private_attributes = True # Filter out the ones that will be deleted by mesher (only when reliable) - if not param_info.entity_transformation_detected: + if not param_info.entity_transformation_detected and not has_missing_private_attributes: # pylint:disable=protected-access,duplicate-code asset_boundary_entities = [ item @@ -431,7 +449,7 @@ def _collect_asset_boundary_entities(params, param_info: ParamsValidationInfo) - params.meshing.volume_zones[0].domain_type, ) - return asset_boundary_entities + return asset_boundary_entities, has_missing_private_attributes def _collect_zone_zone_interfaces( @@ -489,13 +507,14 @@ def _collect_used_boundary_names(params, param_info: ParamsValidationInfo) -> se return used_boundaries -def _validate_boundary_completeness( +def _validate_boundary_completeness( # pylint:disable=too-many-arguments *, asset_boundaries: set, used_boundaries: set, potential_zone_zone_interfaces: set, snappy_multizone: bool, entity_transformation_detected: bool, + has_missing_private_attributes: bool = False, ) -> None: """Validate missing/unknown boundary references with error/warning policy.""" missing_boundaries = asset_boundaries - used_boundaries - potential_zone_zone_interfaces @@ -503,13 +522,17 @@ def _validate_boundary_completeness( if missing_boundaries and not snappy_multizone: missing_list = ", ".join(sorted(missing_boundaries)) - message = ( - f"The following boundaries do not have a boundary condition: {missing_list}. " - "Please add them to a boundary condition model in the `models` section." - ) - if entity_transformation_detected: + if entity_transformation_detected or has_missing_private_attributes: + message = ( + f"The following boundaries do not have a boundary condition: {missing_list}. " + "If these boundaries are valid, please add them to a boundary condition model in the `models` section." + ) add_validation_warning(message) else: + message = ( + f"The following boundaries do not have a boundary condition: {missing_list}. " + "Please add them to a boundary condition model in the `models` section." + ) raise ValueError(message) if unknown_boundaries: @@ -529,7 +552,9 @@ def _check_complete_boundary_condition_and_unknown_surface( return params # Step 2: Collect asset boundaries - asset_boundary_entities = _collect_asset_boundary_entities(params, param_info) + asset_boundary_entities, has_missing_private_attributes = _collect_asset_boundary_entities( + params, param_info + ) if asset_boundary_entities is None or asset_boundary_entities == []: raise ValueError("[Internal] Failed to retrieve asset boundaries") @@ -552,6 +577,7 @@ def _check_complete_boundary_condition_and_unknown_surface( potential_zone_zone_interfaces=potential_zone_zone_interfaces, snappy_multizone=snappy_multizone, entity_transformation_detected=param_info.entity_transformation_detected, + has_missing_private_attributes=has_missing_private_attributes, ) return params diff --git a/tests/simulation/params/data/surface_mesh/simulation.json b/tests/simulation/params/data/surface_mesh/simulation.json index 804578ac9..f019989eb 100644 --- a/tests/simulation/params/data/surface_mesh/simulation.json +++ b/tests/simulation/params/data/surface_mesh/simulation.json @@ -344,7 +344,22 @@ "private_attribute_is_interface": null, "private_attribute_tag_key": null, "private_attribute_sub_components": [], - "private_attribute_color": null + "private_attribute_color": null, + "private_attributes": { + "type_name": "SurfacePrivateAttributes", + "bounding_box": [ + [ + 0, + 0, + 0 + ], + [ + 1, + 1, + 1 + ] + ] + } }, { "private_attribute_registry_bucket_name": "SurfaceEntityType", @@ -355,7 +370,22 @@ "private_attribute_is_interface": null, "private_attribute_tag_key": null, "private_attribute_sub_components": [], - "private_attribute_color": null + "private_attribute_color": null, + "private_attributes": { + "type_name": "SurfacePrivateAttributes", + "bounding_box": [ + [ + 0, + 0, + 0 + ], + [ + 1, + 1, + 1 + ] + ] + } }, { "private_attribute_registry_bucket_name": "SurfaceEntityType", @@ -366,7 +396,22 @@ "private_attribute_is_interface": null, "private_attribute_tag_key": null, "private_attribute_sub_components": [], - "private_attribute_color": null + "private_attribute_color": null, + "private_attributes": { + "type_name": "SurfacePrivateAttributes", + "bounding_box": [ + [ + 0, + 0, + 0 + ], + [ + 1, + 1, + 1 + ] + ] + } }, { "private_attribute_registry_bucket_name": "SurfaceEntityType", @@ -377,7 +422,22 @@ "private_attribute_is_interface": null, "private_attribute_tag_key": null, "private_attribute_sub_components": [], - "private_attribute_color": null + "private_attribute_color": null, + "private_attributes": { + "type_name": "SurfacePrivateAttributes", + "bounding_box": [ + [ + 0, + 0, + 0 + ], + [ + 1, + 1, + 1 + ] + ] + } }, { "private_attribute_registry_bucket_name": "SurfaceEntityType", @@ -388,7 +448,22 @@ "private_attribute_is_interface": null, "private_attribute_tag_key": null, "private_attribute_sub_components": [], - "private_attribute_color": null + "private_attribute_color": null, + "private_attributes": { + "type_name": "SurfacePrivateAttributes", + "bounding_box": [ + [ + 0, + 0, + 0 + ], + [ + 1, + 1, + 1 + ] + ] + } }, { "private_attribute_registry_bucket_name": "SurfaceEntityType", @@ -399,7 +474,22 @@ "private_attribute_is_interface": null, "private_attribute_tag_key": null, "private_attribute_sub_components": [], - "private_attribute_color": null + "private_attribute_color": null, + "private_attributes": { + "type_name": "SurfacePrivateAttributes", + "bounding_box": [ + [ + 0, + 0, + 0 + ], + [ + 1, + 1, + 1 + ] + ] + } }, { "private_attribute_registry_bucket_name": "SurfaceEntityType", @@ -410,7 +500,22 @@ "private_attribute_is_interface": null, "private_attribute_tag_key": null, "private_attribute_sub_components": [], - "private_attribute_color": null + "private_attribute_color": null, + "private_attributes": { + "type_name": "SurfacePrivateAttributes", + "bounding_box": [ + [ + 0, + 0, + 0 + ], + [ + 1, + 1, + 1 + ] + ] + } }, { "private_attribute_registry_bucket_name": "SurfaceEntityType", @@ -421,7 +526,22 @@ "private_attribute_is_interface": null, "private_attribute_tag_key": null, "private_attribute_sub_components": [], - "private_attribute_color": null + "private_attribute_color": null, + "private_attributes": { + "type_name": "SurfacePrivateAttributes", + "bounding_box": [ + [ + 0, + 0, + 0 + ], + [ + 1, + 1, + 1 + ] + ] + } }, { "private_attribute_registry_bucket_name": "SurfaceEntityType", @@ -432,7 +552,22 @@ "private_attribute_is_interface": null, "private_attribute_tag_key": null, "private_attribute_sub_components": [], - "private_attribute_color": null + "private_attribute_color": null, + "private_attributes": { + "type_name": "SurfacePrivateAttributes", + "bounding_box": [ + [ + 0, + 0, + 0 + ], + [ + 1, + 1, + 1 + ] + ] + } }, { "private_attribute_registry_bucket_name": "SurfaceEntityType", @@ -443,7 +578,22 @@ "private_attribute_is_interface": null, "private_attribute_tag_key": null, "private_attribute_sub_components": [], - "private_attribute_color": null + "private_attribute_color": null, + "private_attributes": { + "type_name": "SurfacePrivateAttributes", + "bounding_box": [ + [ + 0, + 0, + 0 + ], + [ + 1, + 1, + 1 + ] + ] + } }, { "private_attribute_registry_bucket_name": "SurfaceEntityType", @@ -454,7 +604,22 @@ "private_attribute_is_interface": null, "private_attribute_tag_key": null, "private_attribute_sub_components": [], - "private_attribute_color": null + "private_attribute_color": null, + "private_attributes": { + "type_name": "SurfacePrivateAttributes", + "bounding_box": [ + [ + 0, + 0, + 0 + ], + [ + 1, + 1, + 1 + ] + ] + } }, { "private_attribute_registry_bucket_name": "SurfaceEntityType", @@ -465,7 +630,22 @@ "private_attribute_is_interface": null, "private_attribute_tag_key": null, "private_attribute_sub_components": [], - "private_attribute_color": null + "private_attribute_color": null, + "private_attributes": { + "type_name": "SurfacePrivateAttributes", + "bounding_box": [ + [ + 0, + 0, + 0 + ], + [ + 1, + 1, + 1 + ] + ] + } }, { "private_attribute_registry_bucket_name": "SurfaceEntityType", @@ -476,7 +656,22 @@ "private_attribute_is_interface": null, "private_attribute_tag_key": null, "private_attribute_sub_components": [], - "private_attribute_color": null + "private_attribute_color": null, + "private_attributes": { + "type_name": "SurfacePrivateAttributes", + "bounding_box": [ + [ + 0, + 0, + 0 + ], + [ + 1, + 1, + 1 + ] + ] + } }, { "private_attribute_registry_bucket_name": "SurfaceEntityType", @@ -487,7 +682,22 @@ "private_attribute_is_interface": null, "private_attribute_tag_key": null, "private_attribute_sub_components": [], - "private_attribute_color": null + "private_attribute_color": null, + "private_attributes": { + "type_name": "SurfacePrivateAttributes", + "bounding_box": [ + [ + 0, + 0, + 0 + ], + [ + 1, + 1, + 1 + ] + ] + } } ], "global_bounding_box": [ diff --git a/tests/simulation/params/test_automated_farfield.py b/tests/simulation/params/test_automated_farfield.py index 49cf1780e..5295f7a9a 100644 --- a/tests/simulation/params/test_automated_farfield.py +++ b/tests/simulation/params/test_automated_farfield.py @@ -581,3 +581,63 @@ def test_domain_type_bounding_box_check(): assert errors is not None domain_errors = [e for e in errors if "The model does not cross the symmetry plane" in e["msg"]] assert len(domain_errors) == 1 + + +def test_legacy_asset_missing_private_attributes(): + """Test that missing BCs are downgraded to warnings for legacy assets without private_attributes.""" + # Create surfaces without private_attributes to simulate legacy cloud assets + wall_surface = Surface(name="wall", private_attribute_id="wall-1") + farfield_surface = Surface(name="farfield_boundary", private_attribute_id="farfield-1") + missing_surface = Surface(name="missing_bc", private_attribute_id="missing-1") + + # Explicitly set to None to simulate legacy assets + wall_surface.private_attributes = None + farfield_surface.private_attributes = None + missing_surface.private_attributes = None + + asset_cache = AssetCache( + project_length_unit="m", + use_inhouse_mesher=True, + use_geometry_AI=False, + project_entity_info=SurfaceMeshEntityInfo( + boundaries=[wall_surface, farfield_surface, missing_surface], + ), + ) + + farfield = AutomatedFarfield() + with SI_unit_system: + params = SimulationParams( + operating_condition=AerospaceCondition(velocity_magnitude=1000), + meshing=MeshingParams( + defaults=MeshingDefaults( + boundary_layer_first_layer_thickness=0.001, + boundary_layer_growth_rate=1.1, + ), + volume_zones=[farfield], + ), + models=[ + Wall(surfaces=[wall_surface]), # Only assign BC to wall + Freestream( + surfaces=[farfield_surface] + ), # Assign to farfield_boundary, not missing_bc + ], + private_attribute_asset_cache=asset_cache, + ) + + # Validate directly without set_up_params_for_uploading to preserve our None values + _, errors, warnings = services.validate_model( + params_as_dict=params.model_dump(mode="json", exclude_none=True), + validated_by=services.ValidationCalledBy.LOCAL, + root_item_type="SurfaceMesh", + validation_level="All", + ) + + # Should get warnings, not errors (missing_bc has no BC assigned) + assert errors is None, f"Expected no errors but got: {errors}" + assert warnings is not None + assert any( + "missing_bc" in w.get("msg", "") for w in warnings + ), f"Expected warning about missing_bc, got: {warnings}" + assert any( + "If these boundaries are valid" in w.get("msg", "") for w in warnings + ), f"Expected specific warning message, got: {warnings}" diff --git a/tests/simulation/params/test_validators_params.py b/tests/simulation/params/test_validators_params.py index 4722af023..262948e53 100644 --- a/tests/simulation/params/test_validators_params.py +++ b/tests/simulation/params/test_validators_params.py @@ -599,6 +599,11 @@ def test_BC_geometry(): with open("./data/geometry_metadata_asset_cache_quasi3D.json") as fp: data = json.load(fp) data["private_attribute_asset_cache"]["project_entity_info"]["face_group_tag"] = "groupName" + # Mock private_attributes for all boundaries + for group in data["private_attribute_asset_cache"]["project_entity_info"]["grouped_faces"]: + for face in group: + if "private_attributes" not in face: + face["private_attributes"] = {"bounding_box": [[0, 0, 0], [1, 1, 1]]} asset_cache = AssetCache(**data["private_attribute_asset_cache"]) symmetry_boundary_1 = [item for item in asset_cache.boundaries if item.name == "symmetry11"][0] @@ -843,11 +848,22 @@ def test_incomplete_BC_volume_mesh(): def test_incomplete_BC_surface_mesh(): ##:: Construct a dummy asset cache - wall_1 = Surface(name="wall_1", private_attribute_is_interface=False) - periodic_1 = Surface(name="periodic_1", private_attribute_is_interface=False) - periodic_2 = Surface(name="periodic_2", private_attribute_is_interface=False) - i_exist = Surface(name="i_exist", private_attribute_is_interface=False) - no_bc = Surface(name="no_bc", private_attribute_is_interface=False) + dummy_attrs = SurfacePrivateAttributes(bounding_box=[[0, 0, 0], [1, 1, 1]]) + wall_1 = Surface( + name="wall_1", private_attribute_is_interface=False, private_attributes=dummy_attrs + ) + periodic_1 = Surface( + name="periodic_1", private_attribute_is_interface=False, private_attributes=dummy_attrs + ) + periodic_2 = Surface( + name="periodic_2", private_attribute_is_interface=False, private_attributes=dummy_attrs + ) + i_exist = Surface( + name="i_exist", private_attribute_is_interface=False, private_attributes=dummy_attrs + ) + no_bc = Surface( + name="no_bc", private_attribute_is_interface=False, private_attributes=dummy_attrs + ) i_will_be_deleted = Surface( name="sym_boundary", private_attribute_is_interface=False, @@ -2269,9 +2285,24 @@ def test_beta_mesher_only_features(mock_validation_context): use_inhouse_mesher=True, project_entity_info=SurfaceMeshEntityInfo( boundaries=[ - Surface(name="face1"), - Surface(name="face2"), - Surface(name="face3"), + Surface( + name="face1", + private_attributes=SurfacePrivateAttributes( + bounding_box=[[0, 0, 0], [1, 1, 1]] + ), + ), + Surface( + name="face2", + private_attributes=SurfacePrivateAttributes( + bounding_box=[[0, 0, 0], [1, 1, 1]] + ), + ), + Surface( + name="face3", + private_attributes=SurfacePrivateAttributes( + bounding_box=[[0, 0, 0], [1, 1, 1]] + ), + ), ] ), ),