Skip to content

Commit 48a8860

Browse files
fix(): Prevent RotationVolumeSplitProvider from intercepting unrelated boundaries (#1696)
1 parent 9f45485 commit 48a8860

File tree

2 files changed

+211
-3
lines changed

2 files changed

+211
-3
lines changed

flow360/component/simulation/framework/boundary_split.py

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ class RotationVolumeSplitProvider:
8484

8585
def __init__(self, params: "SimulationParams"):
8686
self._params = params
87+
self._generated_full_names: Set[str] = set()
8788

8889
# region ====private methods====
8990

@@ -112,6 +113,14 @@ def _get_rotation_volumes(self) -> List["RotationVolume"]:
112113

113114
return [zone for zone in volume_zones if isinstance(zone, RotationVolume)]
114115

116+
def has_active_volumes(self) -> bool:
117+
"""Check if there are any rotation volumes with enclosed entities."""
118+
rotation_volumes = self._get_rotation_volumes()
119+
for rotation_volume in rotation_volumes:
120+
if rotation_volume.enclosed_entities:
121+
return True
122+
return False
123+
115124
@staticmethod
116125
def _find_zone_name(rotation_volume: "RotationVolume", zones: dict) -> Optional[str]:
117126
"""Find the zone name for a rotation volume by matching entity name."""
@@ -222,11 +231,19 @@ def get_split_mappings(self, volume_mesh_meta_data: dict) -> Dict[str, List[Boun
222231
rotation_volume, zone_name, boundary_full_names, stationary_base_names, mappings
223232
)
224233

234+
# Cache generated full names for handled_by_provider check
235+
for infos in mappings.values():
236+
for info in infos:
237+
self._generated_full_names.add(info.full_name)
238+
225239
return mappings
226240

227241
def handled_by_provider(self, full_name: str) -> bool:
228-
"""Check if boundary is a __rotating patch (handled by this provider)."""
229-
return "__rotating_" in full_name
242+
"""
243+
Check if boundary is a __rotating patch (handled by this provider).
244+
Only returns True if the boundary was actually generated by this provider.
245+
"""
246+
return full_name in self._generated_full_names
230247

231248

232249
# --- Lookup Table ---
@@ -270,7 +287,19 @@ def from_params(
270287
This factory method automatically creates the RotationVolumeSplitProvider
271288
based on the params.
272289
"""
273-
providers = [RotationVolumeSplitProvider(params)]
290+
# pylint: disable=import-outside-toplevel
291+
from flow360.component.simulation.entity_info import VolumeMeshEntityInfo
292+
293+
providers = []
294+
entity_info = params.private_attribute_asset_cache.project_entity_info
295+
296+
# Skip if using existing volume mesh (split logic already handled in mesh)
297+
# Also skip if no active rotation volumes defined
298+
if not isinstance(entity_info, VolumeMeshEntityInfo):
299+
rotation_provider = RotationVolumeSplitProvider(params)
300+
if rotation_provider.has_active_volumes():
301+
providers.append(rotation_provider)
302+
274303
return cls(volume_mesh_meta_data, providers)
275304

276305
def _build_mapping(self, volume_mesh_meta_data: dict) -> None:
Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
"""
2+
Regression tests for boundary_split.py
3+
"""
4+
5+
from unittest.mock import MagicMock
6+
7+
import pytest
8+
9+
from flow360.component.simulation.entity_info import (
10+
GeometryEntityInfo,
11+
VolumeMeshEntityInfo,
12+
)
13+
from flow360.component.simulation.framework.boundary_split import (
14+
BoundaryNameLookupTable,
15+
RotationVolumeSplitProvider,
16+
SplitType,
17+
)
18+
from flow360.component.simulation.framework.entity_base import EntityList
19+
from flow360.component.simulation.meshing_param.params import MeshingParams
20+
from flow360.component.simulation.meshing_param.volume_params import RotationVolume
21+
from flow360.component.simulation.primitives import Surface
22+
from flow360.component.simulation.simulation_params import SimulationParams
23+
24+
25+
class TestBoundarySplitDefenses:
26+
"""Tests for the multi-layer defense in Boundary Split Logic."""
27+
28+
@pytest.fixture
29+
def mock_simulation_params(self):
30+
"""Create a mock SimulationParams."""
31+
params = MagicMock(spec=SimulationParams)
32+
33+
# Mock asset cache
34+
asset_cache = MagicMock()
35+
asset_cache.project_entity_info = MagicMock(spec=GeometryEntityInfo)
36+
params.private_attribute_asset_cache = asset_cache
37+
38+
# Mock meshing (no RotationVolume by default)
39+
params.meshing = MagicMock(spec=MeshingParams)
40+
params.meshing.volume_zones = []
41+
42+
return params
43+
44+
def _create_mock_rotation_volume(self):
45+
"""Helper to create a valid Mock RotationVolume with enclosed entity."""
46+
mock_entity = MagicMock(spec=Surface)
47+
mock_entity.name = "blade"
48+
49+
rotation_volume = MagicMock(spec=RotationVolume)
50+
rotation_volume.name = "rotatingZone"
51+
rotation_volume.enclosed_entities = MagicMock(spec=EntityList)
52+
rotation_volume.enclosed_entities.stored_entities = [mock_entity]
53+
rotation_volume.stationary_enclosed_entities = None
54+
55+
# Helper for _find_zone_name
56+
rv_entity = MagicMock()
57+
rv_entity.name = "rotatingZone"
58+
rotation_volume.entities = MagicMock()
59+
rotation_volume.entities.stored_entities = [rv_entity]
60+
61+
return rotation_volume
62+
63+
def test_layer_defense_no_active_rotation_volumes(self, mock_simulation_params):
64+
"""
65+
Layer 2 Defense:
66+
If there are NO active rotation volumes in params, the provider should NOT be added.
67+
"""
68+
volume_mesh_meta_data = {"zones": {}}
69+
70+
# Ensure no rotation volumes
71+
mock_simulation_params.meshing.volume_zones = []
72+
73+
lookup_table = BoundaryNameLookupTable.from_params(
74+
volume_mesh_meta_data, mock_simulation_params
75+
)
76+
77+
# Assertion: Provider list should be empty
78+
assert len(lookup_table._providers) == 0
79+
80+
def test_layer_defense_volume_mesh_entity_info(self, mock_simulation_params):
81+
"""
82+
Layer 1 Defense:
83+
If entity info is VolumeMeshEntityInfo, the provider should NOT be added,
84+
even if RotationVolume exists.
85+
"""
86+
volume_mesh_meta_data = {"zones": {}}
87+
88+
# Setup: Valid RotationVolume
89+
mock_simulation_params.meshing.volume_zones = [self._create_mock_rotation_volume()]
90+
91+
# Setup: VolumeMeshEntityInfo
92+
mock_simulation_params.private_attribute_asset_cache.project_entity_info = MagicMock(
93+
spec=VolumeMeshEntityInfo
94+
)
95+
96+
lookup_table = BoundaryNameLookupTable.from_params(
97+
volume_mesh_meta_data, mock_simulation_params
98+
)
99+
100+
# Assertion: Provider list should be empty
101+
assert len(lookup_table._providers) == 0
102+
103+
def test_layer_defense_mismatched_boundary_name(self, mock_simulation_params):
104+
"""
105+
Layer 3 Defense (Strict Check):
106+
Even if the provider IS added (because we have a valid RotationVolume),
107+
it should NOT intercept boundaries that look like rotating boundaries but
108+
were not generated by THIS provider.
109+
"""
110+
# Setup: Valid RotationVolume (so provider WILL be added)
111+
mock_simulation_params.meshing.volume_zones = [self._create_mock_rotation_volume()]
112+
113+
# Mesh metadata: contains a "suspicious" name that does NOT match the generated one
114+
# Generated one would be: rotatingZone/blade__rotating_rotatingZone
115+
suspicious_name = "farfield/sphere.lb8.ugrid__rotating_intersectingCylinder"
116+
117+
volume_mesh_meta_data = {
118+
"zones": {
119+
"farfield": {"boundaryNames": [suspicious_name]},
120+
# We need rotatingZone present so provider doesn't crash during init
121+
"rotatingZone": {"boundaryNames": []},
122+
}
123+
}
124+
125+
lookup_table = BoundaryNameLookupTable.from_params(
126+
volume_mesh_meta_data, mock_simulation_params
127+
)
128+
129+
# Assertion 1: Provider WAS added
130+
assert len(lookup_table._providers) == 1
131+
provider = lookup_table._providers[0]
132+
assert isinstance(provider, RotationVolumeSplitProvider)
133+
134+
# Assertion 2: Suspicious name is NOT handled by provider
135+
assert provider.handled_by_provider(suspicious_name) is False
136+
137+
# Assertion 3: Suspicious name falls through to default logic (ZONE_PREFIX)
138+
base_name = "sphere.lb8.ugrid__rotating_intersectingCylinder"
139+
split_infos = lookup_table.get_split_info(base_name)
140+
assert len(split_infos) == 1
141+
assert split_infos[0].full_name == suspicious_name
142+
assert split_infos[0].split_type == SplitType.ZONE_PREFIX
143+
144+
def test_provider_intercepts_correctly_generated_boundary(self, mock_simulation_params):
145+
"""
146+
Happy Path:
147+
If RotationVolume exists AND the boundary name matches the generated one,
148+
it SHOULD be intercepted.
149+
"""
150+
# Setup: Valid RotationVolume
151+
mock_simulation_params.meshing.volume_zones = [self._create_mock_rotation_volume()]
152+
153+
generated_full_name = "rotatingZone/blade__rotating_rotatingZone"
154+
volume_mesh_meta_data = {
155+
"zones": {
156+
"rotatingZone": {
157+
"boundaryNames": [
158+
"rotatingZone/blade",
159+
generated_full_name,
160+
]
161+
}
162+
}
163+
}
164+
165+
lookup_table = BoundaryNameLookupTable.from_params(
166+
volume_mesh_meta_data, mock_simulation_params
167+
)
168+
169+
# Assertion 1: Provider WAS added
170+
assert len(lookup_table._providers) == 1
171+
provider = lookup_table._providers[0]
172+
173+
# Assertion 2: Generated name IS handled by provider
174+
assert provider.handled_by_provider(generated_full_name) is True
175+
176+
# Assertion 3: Lookup by base name 'blade' returns the generated split info
177+
split_infos = lookup_table.get_split_info("blade")
178+
full_names = [info.full_name for info in split_infos]
179+
assert generated_full_name in full_names

0 commit comments

Comments
 (0)