-
Notifications
You must be signed in to change notification settings - Fork 9
[FXC-4006][FXC-4053][25.6] Add validation for transition model output fields and output frequency settings #1580
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -49,6 +49,7 @@ | |||||
| from flow360.component.simulation.validation.validation_context import ( | ||||||
| ALL, | ||||||
| CASE, | ||||||
| TimeSteppingType, | ||||||
| get_validation_info, | ||||||
| get_validation_levels, | ||||||
| ) | ||||||
|
|
@@ -192,6 +193,20 @@ class _AnimationSettings(_OutputBase): | |||||
| + " 0 is at beginning of simulation.", | ||||||
| ) | ||||||
|
|
||||||
| @pd.field_validator("frequency", "frequency_offset") | ||||||
| @classmethod | ||||||
| def disable_frequecy_settings_in_steady_simulation(cls, value, info: pd.ValidationInfo): | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's make this change to 25.7 instead and write updater for this. |
||||||
| """Disable frequency settings in a steady simulation""" | ||||||
| validation_info = get_validation_info() | ||||||
| if validation_info is None or validation_info.time_stepping != TimeSteppingType.STEADY: | ||||||
| return value | ||||||
| # pylint: disable=unsubscriptable-object | ||||||
| if value != cls.model_fields[info.field_name].default: | ||||||
| raise ValueError( | ||||||
| f"Output {info.field_name} cannot be specified in a steady simulation." | ||||||
| ) | ||||||
| return value | ||||||
|
|
||||||
|
|
||||||
| class _AnimationAndFileFormatSettings(_AnimationSettings): | ||||||
| """ | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -130,6 +130,39 @@ def _check_output_fields_valid_given_turbulence_model(params): | |
| return params | ||
|
|
||
|
|
||
| def _check_output_fields_valid_given_transition_model(params): | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. Please add updater. |
||
| """Ensure that the output fields are consistent with the transition model used.""" | ||
|
|
||
| if not params.models or not params.outputs: | ||
| return params | ||
|
|
||
| transition_model = "None" | ||
| for model in params.models: | ||
| if isinstance(model, Fluid): | ||
| transition_model = model.transition_model_solver.type_name | ||
| break | ||
|
|
||
| if transition_model != "None": | ||
| return params | ||
|
|
||
| transition_output_fields = [ | ||
| "residualTransition", | ||
| "solutionTransition", | ||
| "linearResidualTransition", | ||
| ] | ||
|
|
||
| for output_index, output in enumerate(params.outputs): | ||
| if output.output_type in ("AeroAcousticOutput", "StreamlineOutput"): | ||
| continue | ||
| for item in output.output_fields.items: | ||
| if isinstance(item, str) and item in transition_output_fields: | ||
| raise ValueError( | ||
| f"In `outputs`[{output_index}] {output.output_type}:, {item} is not a valid" | ||
| f" output field when transition model is not used." | ||
| ) | ||
| return params | ||
|
|
||
|
|
||
| def _check_unsteadiness_to_use_aero_acoustics(params): | ||
|
|
||
| if not params.outputs: | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,14 +1,19 @@ | ||||||
| import json | ||||||
| import os | ||||||
| import re | ||||||
|
|
||||||
| import pydantic as pd | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove |
||||||
| import pytest | ||||||
|
|
||||||
| import flow360 as fl | ||||||
| import flow360.component.simulation.units as u | ||||||
| from flow360.component.simulation.framework.param_utils import AssetCache | ||||||
| from flow360.component.simulation.models.solver_numerics import ( | ||||||
| KOmegaSST, | ||||||
| NoneSolver, | ||||||
| SpalartAllmaras, | ||||||
| ) | ||||||
| from flow360.component.simulation.models.surface_models import Wall | ||||||
| from flow360.component.simulation.models.volume_models import Fluid | ||||||
| from flow360.component.simulation.outputs.output_entities import Point | ||||||
| from flow360.component.simulation.outputs.outputs import ( | ||||||
|
|
@@ -17,17 +22,23 @@ | |||||
| IsosurfaceOutput, | ||||||
| ProbeOutput, | ||||||
| SurfaceOutput, | ||||||
| SurfaceProbeOutput, | ||||||
| TimeAverageSurfaceOutput, | ||||||
| VolumeOutput, | ||||||
| ) | ||||||
| from flow360.component.simulation.primitives import Surface | ||||||
| from flow360.component.simulation.services import clear_context | ||||||
| from flow360.component.simulation.services import ( | ||||||
| ValidationCalledBy, | ||||||
| clear_context, | ||||||
| validate_model, | ||||||
| ) | ||||||
| from flow360.component.simulation.simulation_params import SimulationParams | ||||||
| from flow360.component.simulation.time_stepping.time_stepping import Unsteady | ||||||
| from flow360.component.simulation.time_stepping.time_stepping import Steady, Unsteady | ||||||
| from flow360.component.simulation.unit_system import imperial_unit_system | ||||||
| from flow360.component.simulation.user_code.core.types import UserVariable | ||||||
| from flow360.component.simulation.user_code.functions import math | ||||||
| from flow360.component.simulation.user_code.variables import solution | ||||||
| from flow360.component.volume_mesh import VolumeMeshV2 | ||||||
|
|
||||||
|
|
||||||
| @pytest.fixture() | ||||||
|
|
@@ -129,6 +140,57 @@ def test_turbulence_enabled_output_fields(): | |||||
| ) | ||||||
|
|
||||||
|
|
||||||
| def test_transition_model_enabled_output_fields(): | ||||||
| with pytest.raises( | ||||||
| ValueError, | ||||||
| match=re.escape( | ||||||
| "In `outputs`[0] IsosurfaceOutput:, solutionTransition is not a valid output field when transition model is not used." | ||||||
| ), | ||||||
| ): | ||||||
| with imperial_unit_system: | ||||||
| SimulationParams( | ||||||
| models=[Fluid(transition_model_solver=NoneSolver())], | ||||||
| outputs=[ | ||||||
| IsosurfaceOutput( | ||||||
| name="iso", | ||||||
| entities=[Isosurface(name="tmp", field="mut", iso_value=1)], | ||||||
| output_fields=["solutionTransition"], | ||||||
| ) | ||||||
| ], | ||||||
| ) | ||||||
|
|
||||||
| with pytest.raises( | ||||||
| ValueError, | ||||||
| match=re.escape( | ||||||
| "In `outputs`[0] SurfaceProbeOutput:, residualTransition is not a valid output field when transition model is not used." | ||||||
| ), | ||||||
| ): | ||||||
| with imperial_unit_system: | ||||||
| SimulationParams( | ||||||
| models=[Fluid(transition_model_solver=NoneSolver())], | ||||||
| outputs=[ | ||||||
| SurfaceProbeOutput( | ||||||
| name="probe_output", | ||||||
| probe_points=[Point(name="point_1", location=[1, 2, 3] * u.m)], | ||||||
| output_fields=["residualTransition"], | ||||||
| target_surfaces=[Surface(name="fluid/body")], | ||||||
| ) | ||||||
| ], | ||||||
| ) | ||||||
|
|
||||||
| with pytest.raises( | ||||||
| ValueError, | ||||||
| match=re.escape( | ||||||
| "In `outputs`[0] VolumeOutput:, linearResidualTransition is not a valid output field when transition model is not used." | ||||||
| ), | ||||||
| ): | ||||||
| with imperial_unit_system: | ||||||
| SimulationParams( | ||||||
| models=[Fluid(transition_model_solver=NoneSolver())], | ||||||
| outputs=[VolumeOutput(output_fields=["linearResidualTransition"])], | ||||||
| ) | ||||||
|
|
||||||
|
|
||||||
| def test_surface_user_variables_in_output_fields(): | ||||||
| uv_surface1 = UserVariable( | ||||||
| name="uv_surface1", value=math.dot(solution.velocity, solution.CfVec) | ||||||
|
|
@@ -227,3 +289,69 @@ def test_duplicate_surface_usage(): | |||||
| ], | ||||||
| time_stepping=Unsteady(steps=10, step_size=1e-3), | ||||||
| ) | ||||||
|
|
||||||
|
|
||||||
| def test_output_frequecy_settings_in_steady_simulation(): | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| volume_mesh = VolumeMeshV2.from_local_storage( | ||||||
| mesh_id=None, | ||||||
| local_storage_path=os.path.join( | ||||||
| os.path.dirname(__file__), "..", "data", "vm_entity_provider" | ||||||
| ), | ||||||
| ) | ||||||
| with open( | ||||||
| os.path.join( | ||||||
| os.path.dirname(__file__), "..", "data", "vm_entity_provider", "simulation.json" | ||||||
| ), | ||||||
| "r", | ||||||
| ) as fh: | ||||||
| asset_cache_data = json.load(fh).pop("private_attribute_asset_cache") | ||||||
| asset_cache = AssetCache.model_validate(asset_cache_data) | ||||||
| with imperial_unit_system: | ||||||
| params = SimulationParams( | ||||||
| models=[Wall(name="wall", entities=volume_mesh["*"])], | ||||||
| time_stepping=Steady(), | ||||||
| outputs=[ | ||||||
| VolumeOutput( | ||||||
| output_fields=["Mach", "Cp"], | ||||||
| frequency=2, | ||||||
| ), | ||||||
| SurfaceOutput( | ||||||
| output_fields=["Cp"], | ||||||
| entities=volume_mesh["*"], | ||||||
| frequency_offset=10, | ||||||
| ), | ||||||
| ], | ||||||
| private_attribute_asset_cache=asset_cache, | ||||||
| ) | ||||||
|
|
||||||
| params_as_dict = params.model_dump(exclude_none=True, mode="json") | ||||||
| print("haha", params_as_dict) | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove haha :) |
||||||
|
|
||||||
| params, errors, _ = validate_model( | ||||||
| params_as_dict=params_as_dict, | ||||||
| validated_by=ValidationCalledBy.LOCAL, | ||||||
| root_item_type="VolumeMesh", | ||||||
| validation_level="All", | ||||||
| ) | ||||||
|
|
||||||
| expected_errors = [ | ||||||
| { | ||||||
| "loc": ("outputs", 0, "frequency"), | ||||||
| "type": "value_error", | ||||||
| "msg": "Value error, Output frequency cannot be specified in a steady simulation.", | ||||||
| "ctx": {"relevant_for": ["Case"]}, | ||||||
| }, | ||||||
| { | ||||||
| "loc": ("outputs", 1, "frequency_offset"), | ||||||
| "type": "value_error", | ||||||
| "msg": "Value error, Output frequency_offset cannot be specified in a steady simulation.", | ||||||
| "ctx": {"relevant_for": ["Case"]}, | ||||||
| }, | ||||||
| ] | ||||||
| assert len(errors) == len(expected_errors) | ||||||
| for err, exp_err in zip(errors, expected_errors): | ||||||
| assert err["loc"] == exp_err["loc"] | ||||||
| assert err["type"] == exp_err["type"] | ||||||
| assert err["ctx"]["relevant_for"] == exp_err["ctx"]["relevant_for"] | ||||||
| assert err["msg"] == exp_err["msg"] | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.