Change default serialization units - #313
Draft
kylecarow wants to merge 36 commits into
Draft
Conversation
- Fix Vec<SI> conversion: element-wise mapping with into_iter() - Fix Vec<Self> (soc_bal_iter_history): detect Self in type, substitute struct name - Add Mass and Area to quantity map with primary units - Add percent variant for Ratio - Add serde(rename) to helper SI fields so JSON keys map correctly - Copy only serde attrs (not #[has_state] etc.) to helper non-SI fields - Handle #[serde(skip)] fields: omit from helper, use Default::default() in From - Handle default="fn" by preserving original serde attrs on non-SI fields - Normalize unit plural names: to_lowercase() so degree_celsius -> degrees_celsius - Fix TrackedState path to crate:: for in-crate expansion
- Extract serde(alias) from SI fields and add them to the primary unit helper field, so legacy column names (cycGrade, cycMps, cycSecs) are preserved through the macro transformation - Detect serde(deny_unknown_fields) on the original struct and propagate it to the generated helper struct so field strictness is maintained - Add test_cycle_csv_legacy_aliases: verifies cycSecs/cycMps/cycGrade CSV column names still deserialize correctly
- collect_si_field_data() now returns None for fields with #[serde(skip)], routing them through the non-SI skip path (Default::default() in From impl) instead of the SI unit-conversion path. Fixes panic on pwr_for_peak_eff. - Remove deny_unknown_fields propagation to helper struct: it caused 'missing field' errors when loading old YAML files that predate some fields. - Update resource vehicle YAMLs to new unit-suffixed serialization format (eff->eff_ratio, drag_coef->drag_coef_ratio, etc.). Old files still load via the bare-name alias added to each primary unit helper field. Result: 97/101 unit tests pass (+8 vs pre-fix baseline of 89/101); remaining 4 failures are pre-existing (missing field 'i' in cal_and_val thermal YAML, unrelated to this branch).
For si::Ratio fields, the serialized (canonical) name is now the bare
field name (e.g. 'grade', 'eff', 'drag_coef') rather than the
unit-suffixed form ('grade_ratio', etc.).
Deserialization still accepts all three forms:
- 'grade' → ratio (canonical, base units)
- 'grade_ratio' → ratio (alias)
- 'grade_percent' → percent unit, auto-converts
Achieved by:
- serde_attrs_for_si_field: treat 'ratio' same as '' (no rename)
- generate_helper_struct: for Ratio primary unit, rename = bare_name,
alias = field_ratio (instead of the usual swap)
- Re-add deny_unknown_fields propagation to helper struct
Update integration test assertions to expect 'efficiency'/'tracked_efficiency'
in serialized output (not 'efficiency_ratio').
When the original struct has #[serde(default)], the generated helper struct now also gets #[serde(default)] + #[derive(Default)] so that missing fields in the serialized data are handled correctly instead of panicking. In generate_from_impl, TrackedState and bare SI fields use unwrap_or_default() instead of expect() when the struct has a struct-level default. This fixes 4 test failures: - vehicle::vehicle_model::tests::test_resources - vehicle::vehicle_model::tests::test_calibrated_vehicles - simdrive::tests::test_sim_drive_hev_thrml - simdrive::tests::test_sim_drive_hev_thrml_soak All caused by VehicleState (and similar state structs) having #[serde(default)] but the From impl panicking when time/power/etc. fields were absent from the YAML. Also refactored has_serde_deny_unknown_fields to share has_serde_struct_flag helper.
Aliases on SI fields are now routed to the unit variant whose suffix they end with, rather than always going to the primary (base) unit. Examples: alias = "old_power_kilowatts" → kilowatts helper (correct kW→W conversion) alias = "budget_kilowatts" → kilowatts helper alias = "trip_hours" → hours helper alias = "cycGrade" → no unit suffix → primary (ratio) ← unchanged This makes #[serde(alias)] safe for renaming SI fields where the old name encoded a non-base unit, without silently misinterpreting the value. Add RenamedDevice test struct and two new integration tests: test_alias_routes_to_correct_unit_variant test_alias_bare_name_routes_to_base_unit
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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.
Built on #309
1. New canonical serialization units (proc macro:
quantity_config)The following quantities now serialize in practical engineering units instead of the SI base unit. The raw uom float is in the SI base, so each requires a new
serialize_withhelper to convert on write:EnergyPowerPressureSpecificEnergySpecificPowerTemperatureTemperatureIntervalThe Ratio "bare name legacy escape hatch" was also removed — Ratio fields now get the standard
field_ratio/field_percenttreatment like every other SI quantity.2. New
serialize_withhelpers (serde_helpers.rs)Seven new
impl_si_serialize_as!expansions to support the above:energy_as_kilojoules,energy_as_kilowatt_hours,power_as_kilowatts,pressure_as_kilopascals,ratio_as_percent,specific_energy_as_kilowatt_hours_per_kilogram,specific_power_as_kilowatts_per_kilogram,temperature_as_degrees_celsius,temperature_interval_as_degrees_celsius.3. Field-level
#[si_unit(...)]overrides on struct fieldsAcross vehicle_model.rs, chassis.rs, conv.rs,
hev.rs,hvac_*.rs,electric_machine.rs, fuel_converter.rs,fuel_storage.rs,transmission.rs, and reversible_energy_storage.rs — fields where the new global default isn't the right choice are overridden with an explicit#[si_unit(...)]annotation (e.g.#[si_unit(unitless)]for drag/rolling-resistance coefficients that should serialize as bare dimensionless ratios,#[si_unit(percent)]for SOC and efficiency fields).#[si_unit]CyclegradeVec<si::Ratio>percentCycleElementgradesi::RatiopercentCycle.gradeChassisdrag_coefsi::RatiounitlessChassiswheel_rr_coefsi::RatiounitlessChassiswheel_fric_coefsi::RatiounitlessConventionalVehiclealt_effsi::RatiopercentRGWDBStatesoc_fc_on_bufferTrackedState<si::Ratio>percentRESGreedyWithDynamicBuffersspeed_soc_disch_buffer_coeffOption<si::Ratio>percentRESGreedyWithDynamicBuffersspeed_soc_fc_on_buffer_coeffOption<si::Ratio>percentRESGreedyWithDynamicBuffersspeed_soc_regen_buffer_coeffOption<si::Ratio>percentHEVStopStartControlsoc_fc_forced_onOption<si::Ratio>percentHVACSystemForLumpedCabinStatecopTrackedState<Option<si::Ratio>>unitlessHVACSystemForLumpedCabinAndRESStatecopTrackedState<Option<si::Ratio>>unitlessElectricMachineStateeffTrackedState<si::Ratio>percentElectricMachineStateeff_fwd_at_max_inputTrackedState<si::Ratio>percentElectricMachineStateeff_at_max_regenTrackedState<si::Ratio>percentFuelConverterStateeffTrackedState<si::Ratio>percentFuelConverterThermalStateeff_coeffTrackedState<si::Ratio>percentFuelStorageenergy_capacitysi::Energykilowatt_hoursReversibleEnergyStorageenergy_capacitysi::Energykilowatt_hoursReversibleEnergyStoragemin_socsi::RatiopercentReversibleEnergyStoragemax_socsi::RatiopercentReversibleEnergyStorageStatesocTrackedState<si::Ratio>percentReversibleEnergyStorageStatesoc_regen_bufferTrackedState<si::Ratio>percentReversibleEnergyStorageStatesoc_disch_bufferTrackedState<si::Ratio>percentReversibleEnergyStorageStateeffTrackedState<si::Ratio>percentReversibleEnergyStorageStatesohTrackedState<si::Ratio>percentTrackedState<f64>)TransmissionStateeffTrackedState<si::Ratio>percentVehicleStategrade_currTrackedState<si::Ratio>percentThe core distinction between
unitlessandpercentis physical meaning: dimensionless coefficients used in multiplication (drag, rolling resistance, friction, COP) serialize with their raw value and getunitless; fractions that represent a proportion of a maximum (efficiency, SOC, grade) serialize as percentages.4.
sohfield promoted tosi::RatioIn
ReversibleEnergyStorage,sohwas a rawTrackedState<f64>— it's nowTrackedState<si::Ratio>with#[si_unit(percent)], giving it the same dimensional-safety and multi-unit deserialization that all other SI fields have.5. Integration test updates
tests/test_serde_api_integration.rsupdated to match the new canonical key names (e.g.energy_capacity_kilowatt_hoursinstead ofenergy_capacity_joules,min_soc_percentinstead ofmin_soc_ratio).TODO: perhaps fields like friction/drag/rr coefficients should never have been ratios?