Skip to content

feat(FXC-6541): support multiple surface outputs with different frequ…#1969

Open
NasserFlexCompute wants to merge 1 commit intomainfrom
fxc-6541/multiple-surface-outputs
Open

feat(FXC-6541): support multiple surface outputs with different frequ…#1969
NasserFlexCompute wants to merge 1 commit intomainfrom
fxc-6541/multiple-surface-outputs

Conversation

@NasserFlexCompute
Copy link
Copy Markdown
Contributor

@NasserFlexCompute NasserFlexCompute commented Apr 7, 2026

Multiple Surface Outputs — Behavior Reference

Filename construction

  1. Client translator emits a "name" field on each output: "" for a default (uncustomized) name, or the user's custom name. Array is sorted by "name" before serialization.
  2. Solver (Flow360Solver.cpp): builds a suffix per entry
    • if "name" is non-empty → suffix = _<name>
    • else if array size > 1 → suffix = _<index>
    • else (solo default) → no suffix
  3. FieldWriter: expands "surface%s"surface_<patchName> per patch; for writeSingleFile, stripSurfaceName removes the _<patchName> portion, leaving the per-instance base.

Scenario matrix

# Outputs Surfaces Names writeSingleFile Result
1 1 wing default false surface_wing
2 1 wing, tail default false surface_wing, surface_tail
3 1 wing, tail default true ✅ single file: surfaces
4 2 wing (both) coarse, fine false surface_wing_coarse, surface_wing_fine
5 2 wing (both) coarse, fine true ✅ single files: surfaces_coarse, surfaces_fine
6 2 wing, tail default, default false surface_wing_0, surface_tail_1
7 2 wing, tail default, default true ✅ single files: surfaces_0, surfaces_1
8 2 wing, tail a, b false surface_wing_a, surface_tail_b
9 2 wing, tail a, b true ✅ single files: surfaces_a, surfaces_b
10 2 wing (both) default, default any Rule 1: same surface, default names — need unique names
11 2 wing (both) same, same any Rule 1: same surface, duplicate names — need unique names
12 2 wing, tail same, same false surface_wing_same, surface_tail_same (surface disambiguates)
13 2 wing, tail same, same true Rule 2: writeSingleFile duplicate suffix — need unique names
14 2 wing, tail default, "0" true Rule 2: suffix _0 collides (default gets _0 by index fallback)
15 2 wing, tail shared (wsf=T), shared (wsf=F) mixed surface_wing_shared (single file), surface_tail_shared_<patch> (per-patch)
16 3 wing, tail, body a, b, c true ✅ single files: surfaces_a, surfaces_b, surfaces_c
17 2 wing, tail default, "custom" false surface_wing_0, surface_tail_custom
V1 1 wing (legacy, no name key) false surface_wing (solver wraps legacy object in a singleton array)

Validation rules

  • Rule 1 — Shared-surface uniqueness. When multiple instances of the same output type (SurfaceOutput or TimeAverageSurfaceOutput) reference the same surface, every sharing instance must have a unique, non-default name. (Catches Added listing for case/mesh, added results downloading #10, Test async #11.)
  • Rule 2 — writeSingleFile suffix uniqueness. Among instances of the same output type with write_single_file=True, no two may resolve to the same solver-side suffix. The check simulates the solver's suffix logic (_<name> for custom names, _<index> fallback for defaults in multi-entry arrays), so it also catches the edge case where a custom name equals an index a default-named peer would receive. (Catches added error handling for volume mesh list #13, added missing file for tests #14.)

Backward compatibility preserved

Comment thread flow360/component/simulation/outputs/outputs.py Outdated
Comment thread flow360/component/simulation/translator/solver_translator.py Outdated
Comment thread flow360/component/simulation/translator/solver_translator.py
Comment thread flow360/component/simulation/validation/validation_simulation_params.py Outdated
@NasserFlexCompute NasserFlexCompute marked this pull request as ready for review April 10, 2026 18:05
Copilot AI review requested due to automatic review settings April 10, 2026 18:05
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the simulation output model/translator to support multiple SurfaceOutput / TimeAverageSurfaceOutput instances, enabling the same surface to be emitted in multiple surface-output configs with different frequencies/formats/fields, while adding validation rules around naming and updating translator “golden” JSON references accordingly.

Changes:

  • Translate surfaceOutput / timeAverageSurfaceOutput as multiple per-instance solver configs, adding name to each config and sorting configs by name.
  • Add validation requiring unique, non-default name values when the same surface is used by multiple outputs of the same type; add filename-style validation for output names (now also rejects %).
  • Update unit/integration tests and reference solver JSONs to match the new surface output schema.

Reviewed changes

Copilot reviewed 40 out of 40 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/simulation/translator/test_output_translation.py Updates translation tests for multi-instance surface outputs; adds new multi-output scenarios.
tests/simulation/translator/ref/Flow360_XV15HoverMRF.json Updates golden solver JSON: surfaceOutput becomes an array with per-instance name.
tests/simulation/translator/ref/Flow360_xv15_bet_disk_unsteady_hover.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_xv15_bet_disk_unsteady_hover_UDD.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_xv15_bet_disk_steady_hover.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_xv15_bet_disk_steady_airplane.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_xv15_bet_disk_nested_rotation.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_windtunnel.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_user_variable.json Updates golden solver JSON for new surfaceOutput array format (including name).
tests/simulation/translator/ref/Flow360_TurbFlatPlate137x97_BoxTrip.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_symmetryBC.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_porous_media_volume_zone.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_porous_media_box.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_porous_jump.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_plateASI.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_om6Wing.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_om6wing_wall_model.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_om6wing_streamlines.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_om6wing_stopping_criterion_and_moving_statistic.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_om6wing_SST_with_modified_C_sigma_omega1.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_om6wing_SA_with_modified_C_w2.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_om6Wing_SA_with_low_reynolds_correction.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_om6wing_render.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_om6wing_FS_with_vel.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_om6wing_FS_with_vel_expression.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_om6wing_FS_with_turbulence_quantities.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_om6Wing_debug_type.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_om6Wing_debug_point.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_NestedCylindersSRF.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_mirrored_surface_translation.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_heatFluxCylinder.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_ghost_periodic.json Updates golden solver JSON for new surfaceOutput array format.
tests/simulation/translator/ref/Flow360_CHT_three_cylinders.json Updates golden solver JSON for multi-instance surfaceOutput array format.
tests/simulation/service/test_integration_metadata.py Updates integration test to handle surfaceOutput now being indexed (array).
tests/simulation/params/test_validators_output.py Adds/updates validator tests for duplicate surface usage and invalid name values.
tests/simulation/outputs/test_filename_string.py Adds test rejecting % in FileNameString.
flow360/component/simulation/validation/validation_simulation_params.py Implements new duplicate-surface validation logic requiring unique names.
flow360/component/simulation/translator/solver_translator.py Refactors surface-output translation to per-instance configs and sorts by name.
flow360/component/simulation/simulation_params.py Wires the new duplicate-surface validation into SimulationParams.
flow360/component/simulation/outputs/outputs.py Updates SurfaceOutput/TimeAverageSurfaceOutput naming constraints and filename validation behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread flow360/component/simulation/translator/solver_translator.py
Comment thread flow360/component/simulation/outputs/outputs.py Outdated
Comment thread flow360/component/simulation/translator/solver_translator.py
Comment thread flow360/component/simulation/translator/solver_translator.py
Comment thread flow360/component/simulation/translator/solver_translator.py
Comment thread flow360/component/simulation/translator/solver_translator.py
Copilot AI review requested due to automatic review settings April 10, 2026 20:09
Comment thread flow360/component/simulation/translator/solver_translator.py
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 48 out of 48 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread flow360/component/simulation/translator/solver_translator.py
Comment thread tests/simulation/translator/test_output_translation.py
Comment thread flow360/component/simulation/translator/solver_translator.py
@NasserFlexCompute NasserFlexCompute force-pushed the fxc-6541/multiple-surface-outputs branch from d7c4486 to ab52ca2 Compare April 10, 2026 20:18
Copilot AI review requested due to automatic review settings April 10, 2026 20:20
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 49 out of 49 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread flow360/component/simulation/translator/solver_translator.py
Comment thread flow360/component/simulation/translator/solver_translator.py
Comment thread tests/simulation/translator/test_output_translation.py
@NasserFlexCompute NasserFlexCompute force-pushed the fxc-6541/multiple-surface-outputs branch from e5c4db5 to 633afeb Compare April 10, 2026 21:10
Comment thread flow360/component/simulation/translator/solver_translator.py
Copilot AI review requested due to automatic review settings April 16, 2026 20:00
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 43 out of 43 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread flow360/component/simulation/translator/solver_translator.py
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 2621c32. Configure here.

Comment thread tests/simulation/translator/test_output_translation.py
Client-side translator changes paired with the schema changes in the Flex
repo (fxc-6541/multiple-surface-outputs).

- translator/solver_translator.py:
  - translate_surface_output now returns a list of per-instance config dicts
    (one per SurfaceOutput / TimeAverageSurfaceOutput). Extracted per-instance
    logic into _translate_single_surface_output. writeSingleFile and name are
    taken from each instance directly; name is emitted as "" when the output
    uses its class default, so the solver can apply its fallback suffixing.
  - translate_output sorts translated configs by name before wrapping them in
    add_unused_output_settings_for_comparison, producing a deterministic order
    regardless of user input ordering.
  - Type annotations tightened to Type[...] for class parameters.

- Reference translator JSON updated so surfaceOutput / timeAverageSurfaceOutput
  become arrays, with per-entry name and writeSingleFile fields.

- test_output_translation.py: fixtures and assertions updated for the array
  format; new tests cover the primary multi-instance use case, global ordering
  by name, and the single-instance case emitting an array of length 1 for both
  SurfaceOutput and TimeAverageSurfaceOutput.

- test_integration_metadata.py: update lookup to match the new array shape.

The schema-side pieces (model changes, filename validation, the
surface-output naming validator, filename-string tests, and the bulk of
test_validators_output.py) now live in the Flex repo under
share/flow360-schema. Bump the flow360-schema dependency in pyproject.toml
once that PR lands.

Made-with: Cursor
@NasserFlexCompute NasserFlexCompute force-pushed the fxc-6541/multiple-surface-outputs branch from 2621c32 to b53b690 Compare April 17, 2026 17:07
Copy link
Copy Markdown
Collaborator

@benflexcompute benflexcompute left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to wait for schema side to be merged and then update the lock file in the PR for the schema to be updated into this current PR.

Translator will be moved into compute next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants