Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/simlin-engine/CLAUDE.md

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/simlin-engine/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1188,7 +1188,7 @@ mod ast_tests {
macro_spec: None,
};

let units_ctx = crate::units::Context::new(&[], &Default::default()).unwrap();
let units_ctx = crate::units::Context::new(&[], &Default::default()).0;
let model_s0 = ModelStage0::new(
&model_datamodel,
std::slice::from_ref(&dim),
Expand Down Expand Up @@ -1278,7 +1278,7 @@ mod ast_tests {
macro_spec: None,
};

let units_ctx = crate::units::Context::new(&[], &Default::default()).unwrap();
let units_ctx = crate::units::Context::new(&[], &Default::default()).0;
let model_s0 = ModelStage0::new(
&model_datamodel,
&[dim1.clone(), dim2.clone()],
Expand Down
42 changes: 22 additions & 20 deletions src/simlin-engine/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -750,28 +750,30 @@ impl std::fmt::Debug for ParsedVariableResult {
pub fn project_units_context(db: &dyn Db, project: SourceProject) -> crate::units::Context {
let dm_units = source_units_to_datamodel(project.units(db));
let dm_sim_specs = source_sim_specs_to_datamodel(project.sim_specs(db));
match crate::units::Context::new_with_builtins(&dm_units, &dm_sim_specs) {
Ok(ctx) => ctx,
Err(unit_parse_errors) => {
// Accumulate each unit definition parsing error as a
// project-level diagnostic (no model / variable).
for (unit_name, eq_errors) in &unit_parse_errors {
for eq_err in eq_errors {
CompilationDiagnostic(Diagnostic {
model: String::new(),
variable: Some(unit_name.clone()),
error: DiagnosticError::Unit(crate::common::UnitError::DefinitionError(
eq_err.clone(),
None,
)),
severity: DiagnosticSeverity::Error,
})
.accumulate(db);
}
}
Default::default()
// Construction is partial: keep the context built from the valid unit
// declarations and surface each conflicting/duplicate declaration as a
// project-level diagnostic, rather than discarding every unit definition on
// the first conflict. An empty context would lose all alias normalization
// project-wide (yr/year, person/people, model-defined equivalences) and
// re-create a spurious unit-mismatch flood -- the context-layer parallel of
// the inference partial-results fix (GH #614).
let (ctx, unit_parse_errors) =
crate::units::Context::new_with_builtins(&dm_units, &dm_sim_specs);
for (unit_name, eq_errors) in &unit_parse_errors {
for eq_err in eq_errors {
CompilationDiagnostic(Diagnostic {
model: String::new(),
variable: Some(unit_name.clone()),
error: DiagnosticError::Unit(crate::common::UnitError::DefinitionError(
eq_err.clone(),
None,
)),
severity: DiagnosticSeverity::Error,
})
.accumulate(db);
}
}
ctx
}

/// Cached datamodel dimensions -- computed once per project.
Expand Down
2 changes: 1 addition & 1 deletion src/simlin-engine/src/db_analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2244,7 +2244,7 @@ fn reconstruct_implicit_variable(
};
}

let units_ctx = crate::units::Context::new(&[], &Default::default()).unwrap_or_default();
let units_ctx = crate::units::Context::new(&[], &Default::default()).0;
let mut dummy_implicits = Vec::new();
let parsed_imp = crate::variable::parse_var(
dims,
Expand Down
2 changes: 1 addition & 1 deletion src/simlin-engine/src/db_implicit_deps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub(super) fn extract_implicit_var_deps(
return Vec::new();
}

let units_ctx = crate::units::Context::new(&[], &Default::default()).unwrap_or_default();
let units_ctx = crate::units::Context::new(&[], &Default::default()).0;
let converted_dims: Vec<crate::dimensions::Dimension> = dims
.iter()
.map(crate::dimensions::Dimension::from)
Expand Down
2 changes: 1 addition & 1 deletion src/simlin-engine/src/db_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ fn test_parse_source_variable_matches_direct_parse() {

// Parse directly via parse_var for comparison
let dm_var = &project.models[0].variables[0];
let units_ctx = crate::units::Context::new(&[], &Default::default()).unwrap();
let units_ctx = crate::units::Context::new(&[], &Default::default()).0;
let mut implicit_vars = Vec::new();
let direct_result = parse_var(
&project.dimensions,
Expand Down
57 changes: 36 additions & 21 deletions src/simlin-engine/src/db_units.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ pub fn check_model_units(db: &dyn Db, model: SourceModel, project: SourceProject
variables,
errors: None,
implicit: is_stdlib,
is_macro: src_model.macro_spec(db).is_some(),
}
};

Expand Down Expand Up @@ -263,27 +264,41 @@ pub fn check_model_units(db: &dyn Db, model: SourceModel, project: SourceProject
.values()
.any(|var| var.units().is_some());

// Run unit inference.
let inferred_units = crate::units_infer::infer(&models_s1, units_ctx, target_model)
.unwrap_or_else(|err| {
if has_declared_units
&& let crate::common::UnitError::InferenceError { code, .. } = &err
&& *code == ErrorCode::UnitMismatch
{
CompilationDiagnostic(Diagnostic {
model: model_name.clone(),
variable: None,
error: DiagnosticError::Model(crate::common::Error {
kind: ErrorKind::Model,
code: *code,
details: Some(format!("{}", err)),
}),
severity: DiagnosticSeverity::Warning,
})
.accumulate(db);
}
Default::default()
});
// Run unit inference. Inference is partial: it returns the units it could
// resolve together with any dimensional conflicts it found. We keep the
// resolved units -- so the rest of the model is still unit-checked even when
// one equation conflicts, rather than discarding the whole inferred-units
// map on the first conflict (GH #614).
//
// Conflicts are surfaced as a single umbrella model-level warning rather
// than one diagnostic per conflict: a large macro-instantiated model can
// produce hundreds of internal constraint contradictions, and emitting one
// warning each would flood the report. The full conflict list remains
// available on the `InferenceResult` for callers that want it.
let inference = crate::units_infer::infer(&models_s1, units_ctx, target_model);
if has_declared_units && !inference.conflicts.is_empty() {
let detail = if inference.conflicts.len() == 1 {
format!("{}", inference.conflicts[0])
} else {
format!(
"{} dimensional unit conflicts found during inference; first: {}",
inference.conflicts.len(),
inference.conflicts[0]
)
};
CompilationDiagnostic(Diagnostic {
model: model_name.clone(),
variable: None,
error: DiagnosticError::Model(crate::common::Error {
kind: ErrorKind::Model,
code: ErrorCode::UnitMismatch,
details: Some(detail),
}),
severity: DiagnosticSeverity::Warning,
})
.accumulate(db);
}
let inferred_units = inference.resolved;

// Check stdlib module argument unit compatibility.
//
Expand Down
2 changes: 2 additions & 0 deletions src/simlin-engine/src/db_var_fragment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,8 @@ pub(crate) fn lower_var_fragment(
variables: stage0_vars,
errors: None,
implicit: false,
// Single-variable fragment compilation only; not a macro template.
is_macro: false,
};

let mut models: HashMap<Ident<Canonical>, &crate::model::ModelStage0> = HashMap::new();
Expand Down
4 changes: 2 additions & 2 deletions src/simlin-engine/src/ltm_augment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4297,7 +4297,7 @@ mod tests {
})
};

let units_ctx = crate::units::Context::new(&[], &Default::default()).unwrap();
let units_ctx = crate::units::Context::new(&[], &Default::default()).0;
let mut implicit_vars = Vec::new();
let stage0 = crate::variable::parse_var::<crate::datamodel::ModuleReference, _>(
dims,
Expand Down Expand Up @@ -4568,7 +4568,7 @@ mod tests {

// ApplyToAll target.
let dims = vec![region_dm_dimension()];
let units_ctx = crate::units::Context::new(&[], &Default::default()).unwrap();
let units_ctx = crate::units::Context::new(&[], &Default::default()).0;
let mut implicit = Vec::new();
let a2a_dm = crate::datamodel::Variable::Aux(crate::datamodel::Aux {
ident: "a2a_target".to_string(),
Expand Down
25 changes: 19 additions & 6 deletions src/simlin-engine/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ pub struct ModelStage0 {
/// implicit is true if this model was implicitly added to the project
/// by virtue of it being in the stdlib (or some similar reason)
pub implicit: bool,
/// is_macro is true if this model is a macro definition. A macro is a
/// polymorphic template: its body variables' declared units may name the
/// macro's formal parameters (a Vensim idiom -- e.g. `~ xfrom` inside
/// RAMP FROM TO), so unit inference must treat those as polymorphic rather
/// than concrete base units.
pub is_macro: bool,
}

#[cfg_attr(feature = "debug-derive", derive(Debug))]
Expand All @@ -73,6 +79,10 @@ pub struct ModelStage1 {
/// implicit is true if this model was implicitly added to the project
/// by virtue of it being in the stdlib (or some similar reason)
pub implicit: bool,
/// is_macro is true if this model is a macro definition; see
/// `ModelStage0::is_macro`. Inference treats a macro body's declared units
/// as polymorphic rather than concrete base units.
pub is_macro: bool,
}

#[cfg_attr(feature = "debug-derive", derive(Debug))]
Expand Down Expand Up @@ -964,6 +974,7 @@ impl ModelStage0 {
variables,
errors: None,
implicit,
is_macro: x_model.macro_spec.is_some(),
}
}

Expand Down Expand Up @@ -1065,6 +1076,7 @@ impl ModelStage0 {
variables,
errors: None,
implicit,
is_macro: x_model.macro_spec.is_some(),
}
}
}
Expand Down Expand Up @@ -1110,6 +1122,7 @@ impl ModelStage1 {
model_deps: Some(model_deps),
instantiations: None,
implicit: model_s0.implicit,
is_macro: model_s0.is_macro,
}
}

Expand Down Expand Up @@ -1509,7 +1522,7 @@ fn test_module_parse() {
);

let mut implicit_vars: Vec<datamodel::Variable> = Vec::new();
let units_ctx = crate::units::Context::new(&[], &Default::default()).unwrap();
let units_ctx = crate::units::Context::new(&[], &Default::default()).0;

let owned_models: HashMap<Ident<Canonical>, ModelStage0> = vec![
("main".to_string(), &main_model),
Expand Down Expand Up @@ -1540,7 +1553,7 @@ fn test_module_parse() {

#[test]
fn test_errors() {
let units_ctx = Context::new(&[], &Default::default()).unwrap();
let units_ctx = Context::new(&[], &Default::default()).0;
let main_model = x_model(
"main",
vec![x_aux("aux_3", "unknown_variable * 3.14", None)],
Expand Down Expand Up @@ -1595,7 +1608,7 @@ fn test_errors() {

#[test]
fn test_new_cached_preserves_previous_helper_rewrite() {
let units_ctx = Context::new(&[], &Default::default()).unwrap();
let units_ctx = Context::new(&[], &Default::default()).0;
let main_model = x_model(
"main",
vec![
Expand Down Expand Up @@ -1709,7 +1722,7 @@ fn test_init_expression_vm() {

#[test]
fn test_previous_module_input_var_uses_helper_rewrite() {
let units_ctx = Context::new(&[], &Default::default()).unwrap();
let units_ctx = Context::new(&[], &Default::default()).0;
let module_input = datamodel::Variable::Aux(datamodel::Aux {
ident: "input".to_string(),
equation: datamodel::Equation::Scalar("0".to_string()),
Expand Down Expand Up @@ -1977,7 +1990,7 @@ fn test_all_deps() {
x_aux("aux_4", "mod_1.output", None),
],
);
let units_ctx = Context::new(&[], &Default::default()).unwrap();
let units_ctx = Context::new(&[], &Default::default()).0;
let owned_x_models: HashMap<Ident<Canonical>, ModelStage0> = vec![
("mod_1".to_owned(), &mod_1_model),
("main".to_owned(), &main_model),
Expand Down Expand Up @@ -2026,7 +2039,7 @@ fn test_all_deps() {
};

let mut implicit_vars: Vec<datamodel::Variable> = Vec::new();
let unit_ctx = crate::units::Context::new(&[], &Default::default()).unwrap();
let unit_ctx = crate::units::Context::new(&[], &Default::default()).0;
let mod_1_orig = &main_model.variables[0];
assert_eq!("mod_1", mod_1_orig.get_ident());
let mod_1 = parse_var(&[], mod_1_orig, &mut implicit_vars, &unit_ctx, |mi| {
Expand Down
1 change: 1 addition & 0 deletions src/simlin-engine/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ impl Project {
variables,
errors: None,
implicit: is_stdlib,
is_macro: src_model.macro_spec(db).is_some(),
});
}

Expand Down
6 changes: 3 additions & 3 deletions src/simlin-engine/src/testutils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub(crate) fn x_aux(ident: &str, eqn: &str, units: Option<&str>) -> datamodel::V
pub(crate) fn aux(ident: &str, eqn: &str) -> Variable {
let var = x_aux(ident, eqn, None);
let mut implicit_vars: Vec<datamodel::Variable> = Vec::new();
let unit_ctx = crate::units::Context::new(&[], &Default::default()).unwrap();
let unit_ctx = crate::units::Context::new(&[], &Default::default()).0;
let var = parse_var(&[], &var, &mut implicit_vars, &unit_ctx, |mi| {
Ok(Some(mi.clone()))
});
Expand Down Expand Up @@ -70,7 +70,7 @@ pub(crate) fn x_stock(
pub(crate) fn stock(ident: &str, eqn: &str, inflows: &[&str], outflows: &[&str]) -> Variable {
let var = x_stock(ident, eqn, inflows, outflows, None);
let mut implicit_vars: Vec<datamodel::Variable> = Vec::new();
let unit_ctx = crate::units::Context::new(&[], &Default::default()).unwrap();
let unit_ctx = crate::units::Context::new(&[], &Default::default()).0;
let var = parse_var(&[], &var, &mut implicit_vars, &unit_ctx, |mi| {
Ok(Some(mi.clone()))
});
Expand Down Expand Up @@ -159,7 +159,7 @@ pub(crate) fn x_flow(ident: &str, eqn: &str, units: Option<&str>) -> datamodel::
pub(crate) fn flow(ident: &str, eqn: &str) -> Variable {
let var = x_flow(ident, eqn, None);
let mut implicit_vars: Vec<datamodel::Variable> = Vec::new();
let unit_ctx = crate::units::Context::new(&[], &Default::default()).unwrap();
let unit_ctx = crate::units::Context::new(&[], &Default::default()).0;
let var = parse_var(&[], &var, &mut implicit_vars, &unit_ctx, |mi| {
Ok(Some(mi.clone()))
});
Expand Down
21 changes: 21 additions & 0 deletions src/simlin-engine/src/unit_checking_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,27 @@ mod tests {
.assert_unit_error_vm();
}

#[test]
fn test_rank_result_dimensionless_in_checking() {
// A RANK result is a dimensionless ordinal position, not the units of
// the ranked array. Declaring the result with the array's units
// (`widgets`) is therefore a genuine dimensional error that the unit
// checker must flag -- it must not silently inherit `widgets` from the
// ranked source.
TestProject::new("rank_units_test")
.with_sim_time(0.0, 0.0, 1.0)
.unit("widgets", None)
.indexed_dimension("D", 3)
.array_const_with_units("source[D]", 10.0, "widgets")
.array_aux_direct(
"result",
vec!["D".to_string()],
"RANK(source[D], 1)",
Some("widgets"),
)
.assert_unit_error_vm();
}

#[test]
fn test_delay1_with_units() {
// Test DELAY1 function
Expand Down
Loading
Loading