Skip to content

Salsa incremental path does not propagate unit constraints through stdlib module arguments #421

@bpowers

Description

@bpowers

Description

The salsa incremental compilation path's units_check tracked function does not propagate unit constraints through stdlib module argument mappings. This means unit mismatches in arguments to stdlib modules like SMTH1 (and likely SMOOTH, DELAY, and other stdlib modules) go undetected when using the incremental path.

Concrete example: If a model calls SMTH1(stock_in_widgets, initial_value_in_dollars, smooth_time), the old CompiledProject::from path correctly detected the unit mismatch between the first argument (widgets) and the initial value argument (dollars) via check_units(), which had access to the full compiled model. The salsa path's units_check does not traverse into stdlib module argument bindings, so it silently accepts the mismatch.

This was exposed by test_smth1_unit_mismatch_initial in unit_checking_test.rs, which had to be weakened from assert_unit_error() to assert_compiles_incremental() because the incremental path cannot detect this class of error.

Why it matters

  • Correctness: Unit checking is a key safety net for modelers. Missing unit errors in stdlib module arguments undermines confidence in the unit checker.
  • Parity: The old monolithic compilation path detected these errors; the incremental path should achieve at least the same coverage.
  • User experience: Stdlib modules like SMOOTH and DELAY are heavily used in system dynamics models. Undetected unit mismatches in their arguments could lead to subtle modeling errors.

Components affected

  • src/simlin-engine/src/db.rs -- units_check tracked function
  • src/simlin-engine/src/units_check.rs -- unit checking logic
  • Stdlib module definitions in src/simlin-engine/src/stdlib/

Possible approaches

  1. Extend units_check to resolve stdlib module argument bindings and check that actual argument units are compatible with the module's expected units (mirroring what check_units() did in the monolithic path).
  2. Add a separate salsa tracked function specifically for cross-module unit constraint propagation.

Context

Identified during the interpreter removal work (branch remove-interpreter). The test test_smth1_unit_mismatch_initial in src/simlin-engine/src/unit_checking_test.rs was weakened to accommodate this pre-existing limitation of the incremental path.

Related: #35 (general unit checking tracker -- does not cover this specific incremental-path gap).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions