AI reviewed and modified

Author: NasserFlexCompute

flow360/component/simulation/outputs/outputs.py

@@ -92,7 +92,7 @@
 
 def _validate_filename_string(value: str) -> str:
     """
-    Validate that a string is a valid Linux filename.
+    Validate that a string is safe to use as (or inside) an output filename.
 
     Args:
         value: The string to validate
@@ -101,7 +101,7 @@ def _validate_filename_string(value: str) -> str:
         The validated string
 
     Raises:
-        ValueError: If the string is not a valid filename
+        ValueError: If the string is not safe for use in output filenames
 
     Notes:
         - Disallows forward slash (/) - path separator
@@ -501,6 +501,7 @@ class SurfaceOutput(_AnimationAndFileFormatSettings, _OutputBase):
     @property
     def has_default_name(self) -> bool:
         """Whether this output has no custom name assigned."""
+        # pylint: disable=unsubscriptable-object
         return self.name == type(self).model_fields["name"].default
 
     @contextual_field_validator("entities", mode="after")

flow360/component/simulation/simulation_params.py

@@ -120,12 +120,12 @@
     _check_duplicate_actuator_disk_cylinder_names,
     _check_duplicate_entities_in_models,
     _check_duplicate_isosurface_names,
-    _check_duplicate_surface_usage_in_surface_output,
     _check_hybrid_model_to_use_zonal_enforcement,
     _check_krylov_solver_restrictions,
     _check_low_mach_preconditioner_output,
     _check_numerical_dissipation_factor_output,
     _check_parent_volume_is_rotating,
+    _check_surface_output_naming,
     _check_time_average_output,
     _check_tpg_not_with_isentropic_solver,
     _check_unique_selector_names,
@@ -508,8 +508,9 @@ def check_duplicate_isosurface_names(cls, outputs):
     @contextual_field_validator("outputs", mode="after")
     @classmethod
     def check_duplicate_surface_usage(cls, outputs, param_info: ParamsValidationInfo):
-        """Require unique non-default names when the same surface appears in multiple outputs"""
-        return _check_duplicate_surface_usage_in_surface_output(outputs, param_info)
+        """Validate surface output naming: unique names for outputs sharing a
+        surface, and unique file-suffixes for outputs with ``write_single_file=True``."""
+        return _check_surface_output_naming(outputs, param_info)
 
     @contextual_field_validator("outputs", mode="after")
     @classmethod

flow360/component/simulation/translator/solver_translator.py

@@ -609,9 +609,10 @@ def translate_surface_output(
     """Translate surface output settings.
 
     Returns a list of per-instance solver config dicts (one per SurfaceOutput instance).
+    ``translated`` is required only as a precondition guard: boundaries must already
+    be translated before surface outputs so downstream lookups resolve correctly.
     """
-
-    assert "boundaries" in translated  #  "Boundaries must be translated before surface output"
+    assert "boundaries" in translated, "Boundaries must be translated before surface output"
 
     return [
         _translate_single_surface_output(obj, surface_output_class)

flow360/component/simulation/validation/validation_simulation_params.py

@@ -731,16 +731,24 @@ def _check_duplicate_isosurface_names(outputs):
     return outputs
 
 
-def _check_duplicate_surface_usage_in_surface_output(outputs, param_info: ParamsValidationInfo):
-    """Validate surface output naming:
-    1. When the same surface appears in multiple outputs of the same type,
-       all those outputs must have unique, non-default names.
-    2. Across all instances of the same type, no two custom (non-default)
-       names may be identical (prevents writeSingleFile filename collisions)."""
+def _check_surface_output_naming(outputs, param_info: ParamsValidationInfo):
+    """Validate ``SurfaceOutput`` / ``TimeAverageSurfaceOutput`` naming.
+
+    Rule 1 (shared-surface uniqueness): When the same surface appears in multiple
+    instances of the same output type, every sharing instance must carry a unique,
+    non-default ``name``.
+
+    Rule 2 (write-single-file uniqueness): Among instances of the same output type
+    with ``write_single_file=True``, no two may resolve to the same solver filename
+    suffix. The solver assigns ``_<name>`` for custom names and ``_<index>`` for
+    default-named outputs when more than one instance exists, so Rule 2 catches
+    both duplicate custom names and the edge case where a custom name equals an
+    index that a default-named peer would receive.
+    """
     if outputs is None:
         return outputs
 
-    def _check_surface_usage(
+    def _check_shared_surface_uniqueness(
         outputs, output_type: Union[Type[SurfaceOutput], Type[TimeAverageSurfaceOutput]]
     ):
         surface_to_outputs: dict[str, list[SurfaceOutput]] = {}
@@ -768,29 +776,40 @@ def _check_surface_usage(
                     "output instance that shares the same surface."
                 )
 
-    def _check_write_single_file_name_uniqueness(
+    def _check_write_single_file_suffix_uniqueness(
         outputs, output_type: Union[Type[SurfaceOutput], Type[TimeAverageSurfaceOutput]]
     ):
-        single_file_names = [
-            o.name
-            for o in outputs
-            if is_exact_instance(o, output_type)
-            and o.write_single_file
-            and not o.has_default_name
-        ]
-        if len(single_file_names) != len(set(single_file_names)):
-            seen = set()
-            dupes = sorted({n for n in single_file_names if n in seen or seen.add(n)})
+        instances = [o for o in outputs if is_exact_instance(o, output_type)]
+        if not any(o.write_single_file for o in instances):
+            return
+
+        # Mirror the translator's sort key (empty strings first, then by name)
+        # so the indices line up with what the solver will see.
+        sorted_instances = sorted(instances, key=lambda o: "" if o.has_default_name else o.name)
+
+        suffix_to_outputs: dict[str, list] = {}
+        for index, output in enumerate(sorted_instances):
+            if output.has_default_name:
+                # Solver fallback: only append an index when the array has more than one entry.
+                suffix = f"_{index}" if len(sorted_instances) > 1 else ""
+            else:
+                suffix = f"_{output.name}"
+            if output.write_single_file:
+                suffix_to_outputs.setdefault(suffix, []).append(output)
+
+        colliding = sorted(s for s, outs in suffix_to_outputs.items() if len(outs) > 1)
+        if colliding:
             raise ValueError(
                 f"Multiple `{output_type.__name__}` instances with `write_single_file=True` "
-                f"share the same name: {dupes}. When single-file output is enabled, "
-                "each instance must have a unique name to avoid filename collisions."
+                f"resolve to the same output filename suffix(es): {colliding}. "
+                "Pick unique non-default names, and avoid names that look like an array "
+                "index of a default-named peer (e.g. `0`, `1`)."
             )
 
-    _check_surface_usage(outputs, SurfaceOutput)
-    _check_surface_usage(outputs, TimeAverageSurfaceOutput)
-    _check_write_single_file_name_uniqueness(outputs, SurfaceOutput)
-    _check_write_single_file_name_uniqueness(outputs, TimeAverageSurfaceOutput)
+    _check_shared_surface_uniqueness(outputs, SurfaceOutput)
+    _check_shared_surface_uniqueness(outputs, TimeAverageSurfaceOutput)
+    _check_write_single_file_suffix_uniqueness(outputs, SurfaceOutput)
+    _check_write_single_file_suffix_uniqueness(outputs, TimeAverageSurfaceOutput)
 
     return outputs
 

surface-output-scenarios.md

@@ -447,7 +447,7 @@ def test_duplicate_surface_usage(mock_validation_context):
     # Same custom name on different surfaces WITH writeSingleFile should fail
     with (
         mock_validation_context,
-        pytest.raises(ValueError, match=re.escape("write_single_file=True")),
+        pytest.raises(ValueError, match="resolve to the same output filename suffix"),
     ):
         with imperial_unit_system:
             SimulationParams(
@@ -572,6 +572,137 @@ def test_surface_output_name_invalid_filename():
         )
 
 
+def test_has_default_name_property():
+    """``has_default_name`` correctly identifies outputs using the type-specific default."""
+    default_surface = SurfaceOutput(
+        entities=Surface(name="fluid/body"),
+        output_fields=["Cp"],
+    )
+    named_surface = SurfaceOutput(
+        name="custom",
+        entities=Surface(name="fluid/body"),
+        output_fields=["Cp"],
+    )
+    default_time_avg = TimeAverageSurfaceOutput(
+        entities=Surface(name="fluid/body"),
+        output_fields=["Cp"],
+    )
+    named_time_avg = TimeAverageSurfaceOutput(
+        name="custom",
+        entities=Surface(name="fluid/body"),
+        output_fields=["Cp"],
+    )
+    assert default_surface.has_default_name is True
+    assert named_surface.has_default_name is False
+    assert default_time_avg.has_default_name is True
+    assert named_time_avg.has_default_name is False
+
+    # Setting a SurfaceOutput's name to the TimeAverageSurfaceOutput default
+    # should NOT count as default — the property must consult the subclass default.
+    surface_with_timeavg_default = SurfaceOutput(
+        name="Time average surface output",
+        entities=Surface(name="fluid/body"),
+        output_fields=["Cp"],
+    )
+    assert surface_with_timeavg_default.has_default_name is False
+
+
+def test_write_single_file_time_average_surface_output(mock_validation_context):
+    """Rule 2 also applies to TimeAverageSurfaceOutput."""
+    with (
+        mock_validation_context,
+        pytest.raises(ValueError, match="resolve to the same output filename suffix"),
+    ):
+        with imperial_unit_system:
+            SimulationParams(
+                outputs=[
+                    TimeAverageSurfaceOutput(
+                        name="shared",
+                        entities=Surface(name="fluid/wing"),
+                        output_fields=["Cp"],
+                        write_single_file=True,
+                    ),
+                    TimeAverageSurfaceOutput(
+                        name="shared",
+                        entities=Surface(name="fluid/tail"),
+                        output_fields=["Cp"],
+                        write_single_file=True,
+                    ),
+                ],
+                time_stepping=Unsteady(steps=10, step_size=1e-3),
+            )
+
+
+def test_write_single_file_default_plus_numeric_name_collision(mock_validation_context):
+    """Edge case: default-named output + custom-named ``"0"`` both with
+    ``write_single_file=True`` would collide at the solver's ``multiPatchDataMap``
+    because the translator sorts defaults first (index 0 → suffix ``_0``) and the
+    custom ``"0"`` name also produces suffix ``_0``."""
+    with (
+        mock_validation_context,
+        pytest.raises(ValueError, match="resolve to the same output filename suffix"),
+    ):
+        with imperial_unit_system:
+            SimulationParams(
+                outputs=[
+                    SurfaceOutput(
+                        entities=Surface(name="fluid/wing"),
+                        output_fields=["Cp"],
+                        write_single_file=True,
+                    ),
+                    SurfaceOutput(
+                        name="0",
+                        entities=Surface(name="fluid/tail"),
+                        output_fields=["Cp"],
+                        write_single_file=True,
+                    ),
+                ],
+            )
+
+
+def test_write_single_file_asymmetric_flag_with_same_name(mock_validation_context):
+    """Same custom name is allowed when only one output has ``write_single_file=True``
+    (only that one contributes to the single-file map)."""
+    with mock_validation_context, imperial_unit_system:
+        SimulationParams(
+            outputs=[
+                SurfaceOutput(
+                    name="shared",
+                    entities=Surface(name="fluid/wing"),
+                    output_fields=["Cp"],
+                    write_single_file=True,
+                ),
+                SurfaceOutput(
+                    name="shared",
+                    entities=Surface(name="fluid/tail"),
+                    output_fields=["Cp"],
+                    write_single_file=False,
+                ),
+            ],
+        )
+
+
+def test_write_single_file_default_names_on_disjoint_surfaces(mock_validation_context):
+    """Two default-named SurfaceOutputs on disjoint surfaces with
+    ``write_single_file=True`` are allowed — the solver assigns distinct
+    index-based suffixes ``_0`` and ``_1``."""
+    with mock_validation_context, imperial_unit_system:
+        SimulationParams(
+            outputs=[
+                SurfaceOutput(
+                    entities=Surface(name="fluid/wing"),
+                    output_fields=["Cp"],
+                    write_single_file=True,
+                ),
+                SurfaceOutput(
+                    entities=Surface(name="fluid/tail"),
+                    output_fields=["Cp"],
+                    write_single_file=True,
+                ),
+            ],
+        )
+
+
 def test_check_moving_statistic_applicability_steady_valid():
     """Test moving_statistic with steady simulation - valid case."""
     wall_1 = Wall(entities=Surface(name="fluid/wing"))

tests/simulation/params/test_validators_output.py

@@ -447,6 +447,28 @@ def test_single_surface_output_emits_array():
     assert "wing" in translated["surfaceOutput"][0]["surfaces"]
 
 
+def test_single_time_average_surface_output_emits_array():
+    """A single TimeAverageSurfaceOutput should also produce an array."""
+    with SI_unit_system:
+        param = SimulationParams(
+            time_stepping=Unsteady(step_size=0.1 * u.s, steps=10),
+            outputs=[
+                TimeAverageSurfaceOutput(
+                    entities=[Surface(name="wing")],
+                    output_fields=["Cp"],
+                    output_format="paraview",
+                ),
+            ],
+        )
+    translated = {"boundaries": {}}
+    translated = translate_output(param, translated)
+
+    assert isinstance(translated["timeAverageSurfaceOutput"], list)
+    assert len(translated["timeAverageSurfaceOutput"]) == 1
+    assert translated["timeAverageSurfaceOutput"][0]["name"] == ""
+    assert "wing" in translated["timeAverageSurfaceOutput"][0]["surfaces"]
+
+
 @pytest.fixture()
 def slice_output_config(vel_in_km_per_hr):
     return (