You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
After four root-cause fixes on branch fix-clearn-unit-errors reduced the C-LEARN unit-diagnostic flood from 481 spurious diagnostics to ~14 (fixes: Context self-alias registration, units_infer time-unit alias resolution, units_check tolerating unknown dependency units, and skipping macro-marked models during unit checking), C-LEARN still produces ~14 unit warnings that genuine Vensim does not flag as mismatches. These break down as:
6-7computed units 'gtonsc/degreesc/yr' don't match specified units on permafrost/clathrate methane-flux variables (flux_c_from_permafrost_release, ch4_emissions_from_permafrost_and_clathrate) -- a temperature-sensitivity-coefficient unit that does not fully cancel.
6expected left and right argument units to match, but 'dmnl' and '1/ppm' don't on the ph variable (a log/concentration formula).
1IF branches have different units: then '1/gtonsco2', else '1/yr' on global_annual_rate_of_co2eq_2050_to_2100.
Simlin being legitimately stricter than Vensim -- e.g. Vensim may not check IF-branch unit consistency, or may not require the argument of a LOG/concentration formula to be dimensionless. If so, we may want to relax those specific checks for Vensim parity (an IF-branch-units check and a log-argument-dimensionlessness check are the two candidate relaxations).
Genuine dimensional inconsistencies in the C-LEARN model that Vensim happens to tolerate silently. If so, the diagnostics are correct and the resolution is to document them as known model-authoring imprecision (not an engine change).
Resolving this requires per-variable investigation of the C-LEARN equations against Vensim's actual unit-checking semantics for each of the three distinct message classes (coefficient non-cancellation, log-argument dimensionlessness, IF-branch consistency).
Why it matters
Unit checking is meant to be a verification safety net; residual noise on a flagship real-world model (C-LEARN) erodes trust in it and can mask genuine errors.
If Simlin is over-strict relative to Vensim, every imported Vensim model with these idioms inherits spurious diagnostics.
If these are genuine model errors, surfacing them is correct behavior and the work is documentation, not code.
Components affected
src/simlin-engine/src/units_check.rs -- the IF-branch consistency check and the log/binary-operator argument-units checks
src/simlin-engine/src/units_infer.rs -- coefficient unit cancellation during inference
Reproduce
cargo test -p simlin-engine --test clearn_unit_errors -- --ignored --nocapture dump_clearn_unit_diagnostics
Context
Discovered while fixing the C-LEARN unit-error flood on branch fix-clearn-unit-errors (481 -> 14 diagnostics via four root-cause fixes). These ~14 are explicitly out of scope for that branch -- they require a Vensim-parity semantics decision per message class, not a mechanical fix. Distinct from #454 (resource_unit/resource_units alias), #455 (year-vs-implicit-time stock unification), #456 (duplicate_unit per stdlib instance), #457 (stdlib module output units not propagated), and #474 (the order-dependence that makes the umbrella diagnostic nondeterministic). Related: #35 (general unit-checking tracker).
Description
After four root-cause fixes on branch
fix-clearn-unit-errorsreduced the C-LEARN unit-diagnostic flood from 481 spurious diagnostics to ~14 (fixes: Context self-alias registration,units_infertime-unit alias resolution,units_checktolerating unknown dependency units, and skipping macro-marked models during unit checking), C-LEARN still produces ~14 unit warnings that genuine Vensim does not flag as mismatches. These break down as:computed units 'gtonsc/degreesc/yr' don't match specified unitson permafrost/clathrate methane-flux variables (flux_c_from_permafrost_release,ch4_emissions_from_permafrost_and_clathrate) -- a temperature-sensitivity-coefficient unit that does not fully cancel.expected left and right argument units to match, but 'dmnl' and '1/ppm' don'ton thephvariable (a log/concentration formula).IF branches have different units: then '1/gtonsco2', else '1/yr'onglobal_annual_rate_of_co2eq_2050_to_2100.unit inference [...]: unit_mismatchwarning (which underlying conflict it reports is nondeterministic -- see engine: wrld3_resource_unit_alias_should_not_conflict is order-dependent / flaky #474 for the order-dependence root cause).The open question
It is not yet established whether these are:
LOG/concentration formula to be dimensionless. If so, we may want to relax those specific checks for Vensim parity (an IF-branch-units check and a log-argument-dimensionlessness check are the two candidate relaxations).Resolving this requires per-variable investigation of the C-LEARN equations against Vensim's actual unit-checking semantics for each of the three distinct message classes (coefficient non-cancellation, log-argument dimensionlessness, IF-branch consistency).
Why it matters
Components affected
src/simlin-engine/src/units_check.rs-- the IF-branch consistency check and the log/binary-operator argument-units checkssrc/simlin-engine/src/units_infer.rs-- coefficient unit cancellation during inferenceReproduce
Context
Discovered while fixing the C-LEARN unit-error flood on branch
fix-clearn-unit-errors(481 -> 14 diagnostics via four root-cause fixes). These ~14 are explicitly out of scope for that branch -- they require a Vensim-parity semantics decision per message class, not a mechanical fix. Distinct from #454 (resource_unit/resource_units alias), #455 (year-vs-implicit-time stock unification), #456 (duplicate_unit per stdlib instance), #457 (stdlib module output units not propagated), and #474 (the order-dependence that makes the umbrella diagnostic nondeterministic). Related: #35 (general unit-checking tracker).