-
Notifications
You must be signed in to change notification settings - Fork 18
Description
Summary
The generate_layout function's db_state path (when called with Some(db_state)) may not correctly handle module-output dependencies for systems format models. When dependencies are extracted from the salsa AST rather than from equation text, module output references like module.actual are not normalized to just the module identifier before the all_idents.contains(d) filter at layout/mod.rs:2319. Since all_idents contains only the module name (e.g., module), not the dotted output form (e.g., module.actual), these dependencies get silently dropped.
Why it matters
This causes incorrect dependency graph construction in the layout engine, which can lead to suboptimal or incorrect layout placement of variables that depend on module outputs. Currently the impact is limited because existing systems format tests pass None for db_state, exercising only the string-heuristic path. However, this would become a correctness issue if/when systems format models use the full compilation path for layout generation.
Components affected
src/simlin-engine/src/layout/mod.rs-- theall_idents.contains(d)filter at line 2319
Concrete example
Given a variable foo whose equation references module.actual, the salsa-extracted dependency list includes the string "module.actual". The all_idents set is built from the model's variable names and contains "module" but not "module.actual". The filter at line 2319 drops "module.actual", so the dependency from foo to module is lost.
Possible approaches
- Before the
all_idents.contains(d)check, split dotted identifiers on.and check whether the prefix (module name) is inall_idents. - Expand
all_identsto include dotted module-output forms when modules are present in the model. - Apply the same normalization that the string-heuristic path uses, stripping the
.outputsuffix before the filter.
Context
Identified during review of PR #398 (systems format support). The module reference path at line 2261 handles the Variable::Module case separately and correctly checks against src_ident, but the general dependency filter at line 2319 does not account for the dotted form returned by salsa for non-module variables that reference module outputs.