Full unit physical dimension migration into Schema Side#1888
Merged
benflexcompute merged 45 commits intoBenY/CentralizedSchemafrom Mar 17, 2026
Merged
Full unit physical dimension migration into Schema Side#1888benflexcompute merged 45 commits intoBenY/CentralizedSchemafrom
benflexcompute merged 45 commits intoBenY/CentralizedSchemafrom
Conversation
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…#1856) Co-authored-by: benflexcompute <ben@flexcompute.com> Co-authored-by: Ben <106089368+benflexcompute@users.noreply.github.com>
- Delete _Flow360BaseUnit hierarchy, Flow360ConversionUnitSystem, is_flow360_unit, old UnitSystem(pd.BaseModel), _PredefinedUnitSystem, and all predefined subclasses (~1400 lines removed) - Import UnitSystem, predefined systems, unit_system_manager, and BaseSystemType from flow360-schema - SimulationParams.unit_system field now uses UnitSystemConfig - conversion.py: replace unit_converter() with RestrictedUnitSystem - services.py: use _UNIT_SYSTEMS lookup and DeserializationContext - Update tests for removed flow360_acceleration_unit and _PredefinedUnitSystem Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
- Fix SimulationParams.__init__ to handle deserialization path (model_validate) when no unit system context is active - Add angle to RestrictedUnitSystem supported dimensions - Fix UnitSystemConfig subscript: use .resolve() before __getitem__ - Add resolve() to MockUnitSystem in test fixture - Remove deleted _Flow360BaseUnit references from conftest, test_entities, test_unit_system, test_unit_conversions - Add operating_condition to test SimulationParams constructions that need full unit system for _preprocess - Update floating point comparisons to use np.isclose for bet disk tests - Replace flow360_unit_system with SI_unit_system in test generator Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…Y/FullUnitMigration
…Y/FullUnitMigration
Resolve JSON ref file conflicts by keeping branch content changes and re-sorting keys with tools/sort_ref_json.py. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…Y/FullUnitMigration
…Y/FullUnitMigration
Replace all XxxType.Point/Vector/Axis/Pair/Range/Array references with schema-side equivalents (Length.Vector3, Length.Vector2, etc.) across 10 files (54 non-scalar shape references + 1 AngleType.Positive). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
benflexcompute
force-pushed
the
BenY/CentralizedSchema
branch
from
099f32d to
2c3709d
Compare
- Replace LengthType/AngleType/AngularVelocityType scalar variants with
schema-side equivalents across 6 source files (38 instances)
- Fix validation_context.py to use schema TypeAdapter instead of
LengthType.validate for project_length_unit deserialization
- Fix entity_expansion_utils.py to wrap MirrorStatus.model_validate
in DeserializationContext when deserializing from dict
- Update test assertions for new serialization format (bare SI values
instead of legacy {value, units} dicts)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix solver_translator missing no_unit context for reference_velocity - Replace DeserializationContext+model_validate with deserialize() in source code (entity_expansion_utils, entity_info, services, etc.) - Replace model_validate with deserialize() in tests for serialized data - Update BET disk golden JSON for new serialization format (bare floats) - Update regex patterns for new validator error messages - Fix project_length_unit format in test data (unyt quantity, not string) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ize() Systematic cleanup: wherever DeserializationContext wraps a single model_validate call on serialized data, use the new deserialize() classmethod instead. Keeps DeserializationContext only where it wraps complex flows, TypeAdapter calls, or combined context managers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix snappy _convert_list_to_unyt_array to not wrap bare numbers as dimensionless unyt_array (let schema validators handle unit attachment) - Fix services.py generate_process_json to get mesh_unit from validated asset cache instead of raw dict - Update test assertions for new error loc paths (no trailing 'value') - Update 8 golden JSON ref files for new serialization format - Update test JSON data: project_length_unit "m" -> 1.0 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The model_validator was named 'deserialize' which collides with the new Flow360BaseModel.deserialize() classmethod. Renamed to 'preprocess_variable_declaration' since it's a Pydantic model_validator invoked automatically, not called directly by anyone. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Resolve all 13 merge conflicts keeping FullUnitMigration (ours): - Use .deserialize() over DeserializationContext+model_validate - Keep new schema type imports over old types - Drop duplicate udim registrations (already in schema package) - Drop old _DimensionedType system (~950 lines) - Add pylint disable=no-member for Pydantic field false positives Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Coverage report (flow360)
Click to see where and how coverage changed
The report is truncated to 25 files out of 45. To see the full report, please visit the workflow summary page. This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1996fed601
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The supported set starts with {length, angle} (2 elements) and grows
to 5 when all optional units are provided, but the early-return guard
checked == 4, making it dead code.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…system 1. _init_check_unit_system: use properly resolved UnitSystem in else branch instead of unconditionally overwriting with raw kwarg_unit_system 2. flow360_unit_system: cache RestrictedUnitSystem per-instance to avoid leaking entries into unyt's global registry on every property access. Cache invalidated in _private_set_length_unit when base_length changes. 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.
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.
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
2 participants
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.
Note
Medium Risk
Touches many core simulation/meshing/result models and unit conversion paths; type changes and new deserialization/conversion behavior could surface as runtime validation or unit-handling regressions across existing user params and results workflows.
Overview
Migrates SDK models off
flow360.component.simulation.unit_systemdimension types to schema-side physical-dimension types. MostLengthType/AngleType/etc. usages are replaced withflow360_schema.framework.physical_dimensionsequivalents (e.g.,Length.Vector3,Length.PositiveFloat64,Time.PositiveFloat64) across meshing params, primitives/entities, operating conditions, materials, volume/surface models, and outputs.Updates deserialization and cached-metadata parsing to use schema
deserialize()instead of pydanticmodel_validate(). This affects project utilities, entity info merging, selector/materializer paths, andFlow360BaseModel.from_file().Simplifies unit conversion plumbing and tightens non-dimensionalization behavior.
need_conversion()now treats any unit-carrying value as convertible,unit_converteris removed fromsimulation/conversion.py, and a newRestrictedUnitSystemis introduced to block conversions when base dimensions aren’t defined; case CSV result conversions now build a schema unit system context and call.in_base()directly.Examples updated to import
unytdirectly (import unyt as u) instead of SDK-provided unit aliases.Written by Cursor Bugbot for commit c737897. This will update automatically on new commits. Configure here.