Skip to content

Introduce Gas base class and refactor Air to inherit from it#1958

Open
johnmoore4 wants to merge 1 commit intomainfrom
john/AirToGas
Open

Introduce Gas base class and refactor Air to inherit from it#1958
johnmoore4 wants to merge 1 commit intomainfrom
john/AirToGas

Conversation

@johnmoore4
Copy link
Copy Markdown
Contributor

@johnmoore4 johnmoore4 commented Apr 3, 2026

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.

Note

Medium Risk
Medium risk because it changes core material modeling/serialization (new required gas_constant for Gas, 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 Gas material model with required gas_constant and moves all gas physics methods (pressure, speed of sound, viscosity, gamma) from Air into this generic base, while refactoring Air to inherit from Gas and simply supply air-specific defaults.

Updates simulation plumbing to accept ThermalState.material as a discriminator-based Union[Gas, Air], switches solver translation and TPG validation checks to treat materials as Gas, and adds warnings: one when a Gas uses air’s gas_constant, and a case-level warning that Air may be deprecated.

Refreshes many JSON snapshots to include the now-explicit gas_constant field for air, exports Gas from the top-level package, and adds targeted tests covering Gas construction, Sutherland viscosity, physics equivalence with Air, and serialization roundtrips.

Written by Cursor Bugbot for commit 3b5866c. This will update automatically on new commits. Configure here.

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>
@benflexcompute benflexcompute marked this pull request as ready for review April 3, 2026 19:47
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.

Fix All in Cursor

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)).",
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Fix in Cursor Fix in Web

@benflexcompute
Copy link
Copy Markdown
Collaborator

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 Gas, while Air is still kept as a separate Pydantic model and separate discriminator value. That means we still pay the maintenance cost of two schema identities even though they no longer represent two meaningfully different model types.

My suggestion is:

  • Keep Gas as the only real gas material model
  • Keep Air(...) as a supported user-facing convenience constructor
  • Do not keep Air as a separate schema / separate Pydantic model
  • Use one canonical serialized output: type="gas"
  • Optionally accept historical type="air" input and map it to Gas during parsing

The reasoning is:

  • Gas is the general model
  • Air is easier to use and absolutely still has value
  • but “easy preset” is not the same thing as “different schema”

So I’m not proposing to deprecate Air(...). I’m proposing to keep it as a first-class convenience API, while collapsing the underlying data model to a single gas schema.

In other words:

  • API layer: keep Air(...)
  • model/schema layer: only Gas

That seems cleaner and more self-consistent than keeping both Gas and Air as separate Pydantic model types.

@benflexcompute
Copy link
Copy Markdown
Collaborator

A Plan file sample:

Air/Gas Single-Model Refactor Plan

Generated by: GPT-5 Codex

Goal

Keep Gas as the only real material model for gases.

Keep Air in the repository as a supported convenience entry point for users, but not as a separate schema or a separate Pydantic model.

That means:

  • users can still write Air(...)
  • the runtime object is always Gas
  • internal logic, unions, validators, translators, and docs are centered on Gas

Core Design Position

Gas and Air should not be modeled as two different schemas if their only difference is default values.

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:

  • Gas(...): general, explicit, more flexible
  • Air(...): easier to use, opinionated preset for the common case

So the right separation is:

  • API layer: keep Air(...)
  • data model layer: only Gas
  • schema layer: only one gas schema

Recommended End State

Python API

  • Gas(...) remains the general-purpose API
  • Air(...) remains a supported convenience constructor that returns Gas

Runtime objects

  • all gas materials are instances of Gas

Serialization

  • one canonical output type: type="gas"

Parsing / backward compatibility

  • both type="gas" and type="air" may be accepted as input
  • both parse into the same Gas model
  • re-serialization should normalize to type="gas"

Why this is better than the current PR shape

The current PR already moved almost all gas behavior into Gas, but it still keeps Air as a separate Pydantic model and separate discriminator value.

That leaves the system in an awkward middle state:

  • one runtime behavior model
  • two schema identities
  • two discriminator values treated as first-class outputs
  • duplicated docs and type surface

This keeps the maintenance cost of two models without a meaningful semantic benefit.

Important Clarification

This does not mean Air should be deprecated.

Air still has an important role because it is easier to use than Gas.

So the intended meaning is:

  • Air is a supported shortcut / preset constructor
  • Gas is the underlying general model

Air should remain part of the public API, but it should not remain a separate schema or separate Pydantic model.

Step-by-Step Plan

Step 1. Collapse the schema to a single gas model

Make Gas the only Pydantic model for gas materials.

This means:

  • remove the Air model class
  • remove Air from unions such as Union[Gas, Air, Water]
  • make all internal type expectations refer only to Gas

Acceptance criteria:

  • there is no separate Air model class
  • ThermalState.material only needs Gas or Water
  • FluidMaterialTypes only contains Gas and Water

Step 2. Keep Air(...) as a supported convenience function

Introduce Air(**kwargs) -> Gas as a public constructor helper.

This function should provide the standard air defaults:

  • name="air"
  • gas_constant=287.0529
  • default air Sutherland viscosity
  • default air thermally perfect gas configuration
  • prandtl_number=0.72

It should return:

  • Gas(type="gas", ...)

Acceptance criteria:

  • existing user scripts can still write Air(...)
  • type(Air()) is Gas
  • no internal logic depends on Air being a class

Step 3. Accept "air" as an input alias

At parsing boundaries, allow historical input such as:

  • type="air"

and map it into the canonical Gas model.

Principle:

  • accept historical input
  • normalize to one internal representation
  • emit one canonical serialized form

Acceptance criteria:

  • existing JSON with type="air" still parses
  • the parsed object is Gas
  • re-serialization outputs type="gas"

Step 4. Clean up internal logic and documentation

Remove remaining Air type assumptions throughout the codebase.

This includes:

  • translator logic
  • validator logic
  • operating condition model annotations
  • top-level exports
  • examples
  • docstrings and comments

Acceptance criteria:

  • internal code no longer treats Air as a distinct runtime type
  • comments and docs describe Air as a convenience constructor, not a separate model

Step 5. Update tests and snapshots

Add or update tests for:

  1. Air() returns Gas
  2. Air() produces the same physics as explicit air-parameter Gas(...)
  3. {"type": "air"} parses successfully
  4. historical "air" input re-serializes to canonical "gas"
  5. snapshots converge to a single gas output form

Acceptance criteria:

  • there is only one canonical gas schema in repository outputs
  • there are no longer two official serialized representations for the same model

Final Recommendation

I agree with the design direction that:

  • Air should remain in the repository
  • Air should remain a supported user-facing API
  • Air should not remain a separate schema or separate Pydantic model

The clean design is:

  • one real model: Gas
  • one convenience constructor: Air(...)
  • one canonical output schema: type="gas"
  • optional input alias support for historical type="air" data

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.

See my previous comment

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.

2 participants