Introduce Gas base class and refactor Air to inherit from it#1958
Introduce Gas base class and refactor Air to inherit from it#1958johnmoore4 wants to merge 1 commit intomainfrom
Conversation
Extract generic gas physics from the Air class into a new Gas base class, making the thermally perfect gas (TPG) capability usable for any gas species. - Add Gas class with configurable gas_constant field (required), dynamic_viscosity, thermally_perfect_gas, and prandtl_number. All physics methods (get_speed_of_sound, get_pressure, get_specific_heat_ratio, get_dynamic_viscosity) live on Gas. - Refactor Air to inherit from Gas with air-specific defaults (R=287.0529, Sutherland viscosity, gamma=1.4, Pr=0.72). Full backward compatibility preserved. - Add validation warning on Gas when gas_constant matches air's value (287.0529), alerting users who may have intended to use a different gas. - Add validation warning on Air (at Case-level validation) that Air may be deprecated in a future release in favor of Gas. - Update ThermalState.material type to Union[Gas, Air] with discriminator. - Update isinstance checks in solver_translator and validation to use Gas (Air instances still match via inheritance). - Export Gas from flow360.__init__. - Update 75 JSON snapshot files with new gas_constant field. - Add 7 new tests for Gas class construction, physics, serialization, and Air/Gas equivalence. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| 287.0529 * u.m**2 / u.s**2 / u.K, | ||
| frozen=True, | ||
| description="Specific gas constant for air (287.0529 J/(kg*K)).", | ||
| ) |
There was a problem hiding this comment.
Air gas_constant changed from immutable property to overridable field
Medium Severity
The Air.gas_constant was previously a read-only @property that always returned 287.0529 * u.m**2 / u.s**2 / u.K. Now it's a pydantic Field with frozen=True. While frozen=True prevents mutation after construction, it does not prevent providing a different value during construction or deserialization. This means Air(gas_constant=300 * u.m**2/u.s**2/u.K) or deserializing JSON with a corrupted gas_constant value will silently create an Air with a physically incorrect gas constant. The _warn_if_air_gas_constant validator explicitly skips Air instances via the self.type != "gas" check, so no warning fires either.
|
I think the current PR is very close, but it stops in an awkward middle state. Right now, almost all of the real gas behavior has already been moved into My suggestion is:
The reasoning is:
So I’m not proposing to deprecate In other words:
That seems cleaner and more self-consistent than keeping both |
|
A Plan file sample: Air/Gas Single-Model Refactor PlanGenerated by: GPT-5 Codex GoalKeep Keep That means:
Core Design Position
If they do not differ in behavior, field structure, validation rules, or solver translation semantics, then they are not two different model types. They are one model with two ways of constructing it:
So the right separation is:
Recommended End StatePython API
Runtime objects
Serialization
Parsing / backward compatibility
Why this is better than the current PR shapeThe current PR already moved almost all gas behavior into That leaves the system in an awkward middle state:
This keeps the maintenance cost of two models without a meaningful semantic benefit. Important ClarificationThis does not mean
So the intended meaning is:
Step-by-Step PlanStep 1. Collapse the schema to a single gas modelMake This means:
Acceptance criteria:
Step 2. Keep
|
benflexcompute
left a comment
There was a problem hiding this comment.
See my previous comment


Extract generic gas physics from the Air class into a new Gas base class, making the thermally perfect gas (TPG) capability usable for any gas species.
Note
Medium Risk
Medium risk because it changes core material modeling/serialization (new required
gas_constantforGas, discriminator unions, and translator/validator type checks), which can affect config loading and solver input generation if any edge cases were missed.Overview
Introduces a new
Gasmaterial model with requiredgas_constantand moves all gas physics methods (pressure, speed of sound, viscosity, gamma) fromAirinto this generic base, while refactoringAirto inherit fromGasand simply supply air-specific defaults.Updates simulation plumbing to accept
ThermalState.materialas a discriminator-basedUnion[Gas, Air], switches solver translation and TPG validation checks to treat materials asGas, and adds warnings: one when aGasuses air’sgas_constant, and a case-level warning thatAirmay be deprecated.Refreshes many JSON snapshots to include the now-explicit
gas_constantfield for air, exportsGasfrom the top-level package, and adds targeted tests coveringGasconstruction, Sutherland viscosity, physics equivalence withAir, and serialization roundtrips.Written by Cursor Bugbot for commit 3b5866c. This will update automatically on new commits. Configure here.