Repository navigation
engine: duplicate_unit diagnostics emitted once per stdlib module instance #456
Description
Activity
Additional evidence: this is not specific to
duplicate_unit. The same empty-model + N-fold duplication reproduces withbad_binary_op_in_unitson a different fixture:cargo run -p simlin-cli -- simulate --ltm test/conveyors/covid19_severity.stmx 2>&1 | grep bad_binary_op_in_unitsemits each of these 10 times:
units error in model '' variable 'euros_per_year_per_person': bad_binary_op_in_units units error in model '' variable 'dollars_per_month_per_worker': bad_binary_op_in_units units error in model '' variable 'jobs_per_month_per_worker': bad_binary_op_in_unitsThe original report guessed at
db.rs/stdlib.gen.rs. The actual emission site issrc/simlin-engine/src/db/query.rs:62-74, in the salsa-trackedproject_units_context:model: String::new()is hardcoded, which is where thein model ''comes from.variable: Some(unit_name.clone())puts a unit definition name into thevariableslot.euros_per_year_per_personet al. are project-level unit aliases, not model variables -- so a project-level unit-definition error is being reported through the per-variable diagnostic channel. That confirms the "not model-scoped" hypothesis in the original description, and explains why no UI can navigate to it.- The duplication is the accumulator drain:
project_units_contextaccumulatesCompilationDiagnosticonce, butcollect_all_diagnosticsdrains per model, so a project with N models/module instances re-reports every project-level unit diagnostic N times. The count tracks models/instances, matching the ~20x observed on World3 and the 10x here.
So both halves of this issue live in one place, and the fix has to give project-scoped diagnostics a channel of their own (or at minimum an attribution field distinct from
(model, variable)) plus a single drain. Approach 1 in the description still looks right for theduplicate_unithalf, but it will not by itself fixbad_binary_op_in_units, which is a genuine syntax error in a user unit declaration and must still be reported -- exactly once, attributed to the declaration rather than to a nonexistent model variable.Discovered while fixing #919 (which only touches the severity word and the Error-severity gating of
has_model_errors/has_variable_errors; the attribution and dedup bug here is orthogonal and untouched by that PR).
Description
Each custom unit (
ghectare,hectare,pollution_unit,resource_unit) in World3 is reported asduplicate_unit~20 times -- once per stdlib module instance that gets expanded during compilation.Representative CLI output:
Note the empty model name -- the diagnostic is not attributed to any user model, which strongly suggests the duplicate-unit check is not model-scoped. The repetition count (20 instances for each of the 4 units) matches the number of stdlib module expansions in the model (smth1/smth3/delay1/delay3 sites).
Why it matters
model ''attribution breaks error reporting downstream: a UI can't navigate to a variable in an unnamed model.Components affected
src/simlin-engine/src/db.rs(likelyunits_checktracked function and how it iterates across stdlib models)src/simlin-engine/src/units.rs(unit-context construction, duplicate detection)src/simlin-engine/src/stdlib.gen.rs(stdlib model expansion)Possible approaches
Arc, aligning with Make UnitMap cheaply cloneable via Arc to reduce per-context allocation overhead #318). Do not re-register user-declared units when compiling stdlib models.Option 1 is cleanest and also reduces allocation overhead.
Context
Discovered while fixing the delay3 conflation bug in commit 6d48816. The real CLI surfaces 80 of these diagnostics on a model that declares each unit exactly once, making it a first-impression correctness problem on any model that (a) declares custom units and (b) uses stdlib smooth/delay modules.
Related: #35 (general unit checking), #318 (UnitMap Arc sharing).