feat(FXC-6541): support multiple surface outputs with different frequ…#1969
Open
NasserFlexCompute wants to merge 2 commits intomainfrom
Open
feat(FXC-6541): support multiple surface outputs with different frequ…#1969NasserFlexCompute wants to merge 2 commits intomainfrom
NasserFlexCompute wants to merge 2 commits intomainfrom
Conversation
Contributor
There was a problem hiding this comment.
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/timeAverageSurfaceOutputas multiple per-instance solver configs, addingnameto each config and sorting configs by name. - Add validation requiring unique, non-default
namevalues 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.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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.
NasserFlexCompute
force-pushed
the
fxc-6541/multiple-surface-outputs
branch
from
d7c4486 to
ab52ca2
Compare
Contributor
There was a problem hiding this comment.
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.
NasserFlexCompute
force-pushed
the
fxc-6541/multiple-surface-outputs
branch
from
e5c4db5 to
633afeb
Compare
Contributor
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ 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.
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
force-pushed
the
fxc-6541/multiple-surface-outputs
branch
from
2621c32 to
b53b690
Compare
Collaborator
benflexcompute
left a comment
benflexcompute
left a comment
There was a problem hiding this comment.
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.
benflexcompute
previously approved these changes
Apr 19, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 35 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Coverage report (flow360)
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Scenario matrix
writeSingleFilewingsurface_wingwing, tailsurface_wing,surface_tailwing, tailsurfaceswing(both)coarse,finesurface_wing_coarse,surface_wing_finewing(both)coarse,finesurfaces_coarse,surfaces_finewing,tailsurface_wing,surface_tailwing,tailwriteSingleFile— same empty suffixwing,taila,bsurface_wing_a,surface_tail_bwing,taila,bsurfaces_a,surfaces_bwing(both)wing(both)same,samewing,tailsame,samesurface_wing_same,surface_tail_same(surface disambiguates)wing,tailsame,samewriteSingleFileduplicate suffix — need unique nameswing,tail"0"""and_0are distinctwing,tailshared(wsf=T),shared(wsf=F)wing,tail,bodya,b,csurfaces_a,surfaces_b,surfaces_cwing,tailcustomsurface_wing,surface_tail_customwingnamekey)surface_wing(solver wraps legacy object in a singleton array)Validation rules
SurfaceOutputorTimeAverageSurfaceOutput) reference the same surface, every sharing instance must have a unique, non-defaultname. (Catches Added listing for case/mesh, added results downloading #10, Test async #11.)writeSingleFilesuffix uniqueness. Among instances of the same output type withwrite_single_file=True, no two may resolve to the same filename suffix. Default-named outputs get no suffix; custom-named outputs get_<name>. (Catches Add case #7, added error handling for volume mesh list #13.)Backward compatibility preserved
SurfaceOutputwith one or more surfaces produces identical filenames to master. (add pydantic #1, test pr #2, add http utils #3)writeSingleFile=True/Falsewith the same name on disjoint surfaces works: onlywriteSingleFile=Trueinstances contribute to the single-file collision check. (fix: supporting muRef in freestream #15)namekey) continues to work; the solver wraps it in a singleton array. (V1)Note
Medium Risk
Changes the solver JSON schema for
surfaceOutput/timeAverageSurfaceOutputfrom a single object to an array of per-instance configs, which can break downstream consumers expecting the legacy shape.Overview
Surface output translation now supports multiple independent
SurfaceOutput/TimeAverageSurfaceOutputinstances. The translator emitssurfaceOutputandtimeAverageSurfaceOutputas lists of per-instance solver config dicts (instead of a single merged object), allowing the same surface to be output with different frequencies/formats.Each emitted surface-output config now includes a normalized
name(default name becomes empty string), uses the instance’swriteSingleFile, and the resulting list is sorted bynamefor stable diffs. Tests and reference solver JSON fixtures are updated accordingly (including integration metadata assertions) to index intosurfaceOutput[0]and validate ordering/behavior for multiple outputs.Reviewed by Cursor Bugbot for commit 161acba. Bugbot is set up for automated code reviews on this repo. Configure here.