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
5 changes: 5 additions & 0 deletions src/libsimlin/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,11 @@ impl From<engine::ErrorCode> for SimlinErrorCode {
engine::ErrorCode::DimensionInScalarContext => SimlinErrorCode::Generic,
engine::ErrorCode::BadOverride => SimlinErrorCode::BadOverride,
engine::ErrorCode::UnsupportedForSerialization => SimlinErrorCode::Generic,
// A bare reference to a lookup table (used as a value without an
// argument) is, at the FFI granularity, a generic model error; the
// precise distinction is preserved in the engine-level `ErrorCode`
// and the error's `details` message (issue #606).
engine::ErrorCode::LookupReferencedWithoutArgument => SimlinErrorCode::Generic,
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/simlin-engine/CLAUDE.md

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion src/simlin-engine/src/ast/expr2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -998,7 +998,8 @@ impl Expr2 {
loc = Some(id_loc);
}
}
BuiltinContents::Expr(expr) => {
// The lookup table identity is a locatable reference too.
BuiltinContents::Expr(expr) | BuiltinContents::LookupTable(expr) => {
if loc.is_none() {
loc = expr.get_var_loc(ident);
}
Expand Down
5 changes: 4 additions & 1 deletion src/simlin-engine/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,10 @@ impl LatexVisitor {
walk_builtin_expr(builtin, |contents| {
let arg = match contents {
BuiltinContents::Ident(id, _loc) => format!("\\mathrm{{{id}}}"),
BuiltinContents::Expr(expr) => self.walk(expr),
// The lookup table identity is a printed argument too.
BuiltinContents::Expr(expr) | BuiltinContents::LookupTable(expr) => {
self.walk(expr)
}
};
args.push(arg);
});
Expand Down
11 changes: 10 additions & 1 deletion src/simlin-engine/src/builtins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,15 @@ pub fn is_builtin_fn(name: &str) -> bool {
pub(crate) enum BuiltinContents<'a, Expr> {
Ident(&'a str, Loc),
Expr(&'a Expr),
/// The table-identity argument of a graphical-function lookup
/// (`LOOKUP`/`LOOKUP FORWARD`/`LOOKUP BACKWARD`) -- a reference to a
/// gf-holding variable (a standalone lookup-only table, or a value-bearing
/// WITH LOOKUP variable's own table), NOT a runtime value input. The static
/// table data is laid out at compile time independent of the runlist, so
/// dependency/causal walkers must treat this as a non-edge (a table
/// reference imposes no runtime data dependency); printing and
/// source-location walkers treat it like any other argument expression.
LookupTable(&'a Expr),
}

pub(crate) fn walk_builtin_expr<'a, Expr, F>(builtin: &'a BuiltinFn<Expr>, mut cb: F)
Expand All @@ -470,7 +479,7 @@ where
BuiltinFn::Lookup(table_expr, index_expr, _loc)
| BuiltinFn::LookupForward(table_expr, index_expr, _loc)
| BuiltinFn::LookupBackward(table_expr, index_expr, _loc) => {
cb(BuiltinContents::Expr(table_expr));
cb(BuiltinContents::LookupTable(table_expr));
cb(BuiltinContents::Expr(index_expr));
}
BuiltinFn::Abs(a)
Expand Down
6 changes: 6 additions & 0 deletions src/simlin-engine/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ pub enum ErrorCode {
// appended freely. Keep additions at the END of the enum anyway, to keep
// the existing discriminants stable for any in-memory consumers.
DuplicateMacroName,
/// A standalone lookup-only table (a graphical function with no driving
/// input) was referenced bare -- without applying it to an argument. A
/// table has no scalar value of its own; it must be called, e.g.
/// `LOOKUP(my_table, x)` or `my_table(x)` (issue #606).
LookupReferencedWithoutArgument,
}

impl fmt::Display for ErrorCode {
Expand Down Expand Up @@ -168,6 +173,7 @@ impl fmt::Display for ErrorCode {
BadOverride => "bad_override",
UnsupportedForSerialization => "unsupported_for_serialization",
DuplicateMacroName => "duplicate_macro_name",
LookupReferencedWithoutArgument => "lookup_referenced_without_argument",
};

write!(f, "{name}")
Expand Down
281 changes: 26 additions & 255 deletions src/simlin-engine/src/compiler/mod.rs

Large diffs are not rendered by default.

114 changes: 86 additions & 28 deletions src/simlin-engine/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,41 @@ pub struct SourceVariable {
pub compat: datamodel::Compat,
}

/// Whether a source variable is a standalone lookup-only table: a
/// graphical-function holder with an empty-or-sentinel equation and no real
/// functional input. Such a variable is a *table indexed by an explicit input*
/// (`y = table(input)`), not a value-bearing variable -- it is excluded from the
/// runlist and produces no saved series (issue #606). This is the salsa-layer
/// twin of `crate::variable::var_is_lookup_only`, evaluated over the
/// `SourceEquation` + `SourceGraphicalFunction` representation; both delegate to
/// the shared `crate::variable::is_empty_or_sentinel` core (which also accepts
/// the legacy `"0+0"` sentinel for back-compat).
///
/// Salsa-tracked so its `bool` output backdates: callers in tracked contexts
/// (`build_var_info` -> `model_dependency_graph`, `calc_flattened_offsets`)
/// must NOT gain a fine-grained dependency on a variable's equation TEXT, which
/// would invalidate the dependency graph on every unrelated equation edit.
#[salsa::tracked]
pub(crate) fn source_var_is_table_only(db: &dyn Db, var: SourceVariable) -> bool {
use crate::variable::is_empty_or_sentinel;
match var.equation(db) {
// Scalar / A2A: one equation string plus a variable-level gf.
SourceEquation::Scalar(s) | SourceEquation::ApplyToAll(_, s) => {
var.gf(db).is_some() && is_empty_or_sentinel(s)
}
// Arrayed: a pure per-element table holder iff it has tables (a
// variable-level or any per-element gf) and EVERY element equation (and
// the EXCEPT default, if any) is empty/sentinel.
SourceEquation::Arrayed(_, elements, default, _) => {
let has_tables = var.gf(db).is_some() || elements.iter().any(|e| e.gf.is_some());
has_tables
&& !elements.is_empty()
&& elements.iter().all(|e| is_empty_or_sentinel(&e.equation))
&& default.as_deref().map(is_empty_or_sentinel).unwrap_or(true)
}
}
}

// ── Mirror types for salsa compatibility ───────────────────────────────
//
// These types mirror the datamodel types but derive salsa::Update.
Expand Down Expand Up @@ -861,6 +896,12 @@ pub struct VariableDeps {
pub dt_previous_referenced_vars: BTreeSet<String>,
/// Variables referenced *only* through PREVIOUS(...) in the initial AST.
pub initial_previous_referenced_vars: BTreeSet<String>,
/// Standalone lookup tables referenced via `LOOKUP(table, x)`. These are
/// layout references (codegen needs the table's offset for its reverse-map)
/// but NOT data-flow dependencies, so they are kept out of `dt_deps` /
/// `initial_deps` (no runlist-ordering or causal/LTM edge) and reunited with
/// the dep set only by the fragment compiler's metadata/tables build (#606).
pub referenced_tables: BTreeSet<String>,
}

fn canonical_module_input_set(module_input_names: &[String]) -> BTreeSet<Ident<Canonical>> {
Expand Down Expand Up @@ -892,6 +933,8 @@ fn variable_direct_dependencies_impl(
dt_init_only_referenced_vars: BTreeSet::new(),
dt_previous_referenced_vars: BTreeSet::new(),
initial_previous_referenced_vars: BTreeSet::new(),
// A module never references a lookup table via LOOKUP(...).
referenced_tables: BTreeSet::new(),
}
}
_ => {
Expand Down Expand Up @@ -945,6 +988,11 @@ fn variable_direct_dependencies_impl(
dt_init_only_referenced_vars: dt_classification.init_only,
dt_previous_referenced_vars: dt_classification.previous_only,
initial_previous_referenced_vars: init_classification.previous_only,
referenced_tables: dt_classification
.referenced_tables
.into_iter()
.chain(init_classification.referenced_tables)
.collect(),
}
}
}
Expand Down Expand Up @@ -1276,7 +1324,7 @@ fn init_value_equivalence_group(
use crate::builtins::{BuiltinContents, walk_builtin_expr};
let mut found = false;
walk_builtin_expr(builtin, |c| {
if let BuiltinContents::Expr(inner) = c
if let BuiltinContents::Expr(inner) | BuiltinContents::LookupTable(inner) = c
&& !found
{
found = find_value_branches(inner, &mut *out);
Expand Down Expand Up @@ -4362,6 +4410,10 @@ fn compile_implicit_var_phase_bytecodes(
.dt_deps
.iter()
.chain(iv_deps.initial_deps.iter())
// Lookup tables referenced by this implicit var are layout
// references, not data-flow deps -- include them so the fragment's
// metadata + tables map can resolve `LOOKUP(table, x)` (#606).
.chain(iv_deps.referenced_tables.iter())
.cloned()
.collect()
} else {
Expand Down Expand Up @@ -5696,28 +5748,33 @@ fn calc_flattened_offsets_incremental(

for ident in &sorted_names {
let ident_canonical = Ident::new(ident.as_str());
let size =
if let Some(svar) = source_vars.get(ident.as_str()) {
if svar.kind(db) == SourceVariableKind::Module {
let sub_model_name = svar.model_name(db);
let sub_offsets =
calc_flattened_offsets_incremental(db, project, sub_model_name, false);
let mut sub_var_names: Vec<&Ident<Canonical>> = sub_offsets.keys().collect();
sub_var_names.sort_unstable();
for sub_name in &sub_var_names {
let (sub_off, sub_size) = sub_offsets[*sub_name];
offsets.insert(
Ident::join(
&ident_canonical.as_canonical_str(),
&sub_name.as_canonical_str(),
),
(i + sub_off, sub_size),
);
}
let sub_size: usize = sub_offsets.iter().map(|(_, (_, size))| size).sum();
sub_size
} else {
let var_sz = variable_size(db, *svar, project);
let size = if let Some(svar) = source_vars.get(ident.as_str()) {
if svar.kind(db) == SourceVariableKind::Module {
let sub_model_name = svar.model_name(db);
let sub_offsets =
calc_flattened_offsets_incremental(db, project, sub_model_name, false);
let mut sub_var_names: Vec<&Ident<Canonical>> = sub_offsets.keys().collect();
sub_var_names.sort_unstable();
for sub_name in &sub_var_names {
let (sub_off, sub_size) = sub_offsets[*sub_name];
offsets.insert(
Ident::join(
&ident_canonical.as_canonical_str(),
&sub_name.as_canonical_str(),
),
(i + sub_off, sub_size),
);
}
let sub_size: usize = sub_offsets.iter().map(|(_, (_, size))| size).sum();
sub_size
} else {
let var_sz = variable_size(db, *svar, project);
// A lookup-only table is not a saved output variable: reserve
// its layout slot (so these offsets stay in lockstep with
// `compute_layout`, whose map codegen's table-identity
// reverse-map resolves against) but do NOT expose its name in
// this VM/Results map -- it produces no series (issue #606).
if !source_var_is_table_only(db, *svar) {
if var_sz > 1 {
// Array variable: produce per-element offsets
let dims = variable_dimensions(db, *svar, project);
Expand All @@ -5735,12 +5792,13 @@ fn calc_flattened_offsets_incremental(
} else {
offsets.insert(ident_canonical.clone(), (i, 1));
}
var_sz
}
} else {
offsets.insert(ident_canonical.clone(), (i, 1));
1
};
var_sz
}
} else {
offsets.insert(ident_canonical.clone(), (i, 1));
1
};
i += size;
}

Expand Down
20 changes: 16 additions & 4 deletions src/simlin-engine/src/db_dep_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ use crate::db::model_dependency_graph;
pub(crate) struct VarInfo {
pub(crate) is_stock: bool,
pub(crate) is_module: bool,
/// A standalone lookup-only table: excluded from every runlist and from the
/// saved output, because it is a static table indexed by callers, not a
/// value-bearing variable (issue #606).
pub(crate) is_table_only: bool,
pub(crate) dt_deps: BTreeSet<String>,
pub(crate) initial_deps: BTreeSet<String>,
}
Expand Down Expand Up @@ -324,6 +328,7 @@ pub(crate) fn build_var_info(
VarInfo {
is_stock: kind == SourceVariableKind::Stock,
is_module: kind == SourceVariableKind::Module,
is_table_only: crate::db::source_var_is_table_only(db, source_vars[name.as_str()]),
dt_deps: normalize_deps(&dt_deps),
initial_deps: normalize_deps(&initial_deps),
},
Expand Down Expand Up @@ -353,6 +358,8 @@ pub(crate) fn build_var_info(
VarInfo {
is_stock: implicit.is_stock,
is_module: implicit.is_module,
// Implicit SMOOTH/DELAY/TREND internals are never lookup tables.
is_table_only: false,
dt_deps: normalize_deps(&dt_deps),
initial_deps: normalize_deps(&initial_deps),
},
Expand Down Expand Up @@ -2271,9 +2278,12 @@ pub(crate) fn model_dependency_graph_impl(
var_info
.get(n.as_str())
.map(|i| {
i.is_stock
|| i.is_module
|| (i.dt_deps.is_empty() && !i.initial_deps.is_empty())
// A lookup-only table produces no value, so it is never
// an initials-phase variable (issue #606).
!i.is_table_only
&& (i.is_stock
|| i.is_module
|| (i.dt_deps.is_empty() && !i.initial_deps.is_empty()))
})
.unwrap_or(false)
|| all_init_referenced.contains(n.as_str())
Expand Down Expand Up @@ -2345,7 +2355,9 @@ pub(crate) fn model_dependency_graph_impl(
let is_input = module_input_set.contains(canonicalize(n).as_ref());
var_info
.get(n.as_str())
.map(|i| is_input || !i.is_stock)
// A lookup-only table is a static table, not a flow: it is
// never lowered and emits no bytecode (issue #606).
.map(|i| (is_input || !i.is_stock) && !i.is_table_only)
.unwrap_or(false)
})
.collect();
Expand Down
2 changes: 2 additions & 0 deletions src/simlin-engine/src/db_dep_graph_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ fn vi_for_test(is_stock: bool, is_module: bool, dt_deps: &[&str]) -> VarInfo {
VarInfo {
is_stock,
is_module,
is_table_only: false,
dt_deps: dt_deps.iter().map(|s| (*s).to_string()).collect(),
initial_deps: BTreeSet::new(),
}
Expand Down Expand Up @@ -135,6 +136,7 @@ fn vi_init_for_test(is_stock: bool, is_module: bool, initial_deps: &[&str]) -> V
VarInfo {
is_stock,
is_module,
is_table_only: false,
dt_deps: BTreeSet::new(),
initial_deps: initial_deps.iter().map(|s| (*s).to_string()).collect(),
}
Expand Down
12 changes: 12 additions & 0 deletions src/simlin-engine/src/db_implicit_deps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ pub struct ImplicitVarDeps {
pub dt_init_only_referenced_vars: BTreeSet<String>,
pub dt_previous_referenced_vars: BTreeSet<String>,
pub initial_previous_referenced_vars: BTreeSet<String>,
/// Lookup tables referenced via `LOOKUP(table, x)` -- layout references, not
/// data-flow deps. Kept out of `dt_deps`/`initial_deps` (no ordering/causal
/// edge) but needed by the implicit-var fragment compiler's metadata +
/// tables map (issue #606). Mirrors `VariableDeps::referenced_tables`.
pub referenced_tables: BTreeSet<String>,
}

pub(super) fn extract_implicit_var_deps(
Expand Down Expand Up @@ -63,6 +68,8 @@ pub(super) fn extract_implicit_var_deps(
dt_init_only_referenced_vars: BTreeSet::new(),
dt_previous_referenced_vars: BTreeSet::new(),
initial_previous_referenced_vars: BTreeSet::new(),
// A module never references a lookup table via LOOKUP(...).
referenced_tables: BTreeSet::new(),
};
}

Expand Down Expand Up @@ -115,6 +122,11 @@ pub(super) fn extract_implicit_var_deps(
dt_init_only_referenced_vars: dt_classification.init_only,
dt_previous_referenced_vars: dt_classification.previous_only,
initial_previous_referenced_vars: init_classification.previous_only,
referenced_tables: dt_classification
.referenced_tables
.into_iter()
.chain(init_classification.referenced_tables)
.collect(),
}
})
.collect()
Expand Down
4 changes: 4 additions & 0 deletions src/simlin-engine/src/db_ltm_ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,10 @@ fn walk_all_in_expr(
child_in_reducer,
sites,
),
// A graphical-function table reference is static data, not a
// causal edge: emit no `from -> consumer` reference site for the
// table itself (only the index argument carries real edges).
BuiltinContents::LookupTable(_) => {}
});
}
Expr2::Op1(_, operand, _, _) => {
Expand Down
Loading
Loading