Skip to content

Make validParams()/validSim() cheap on already-valid objects (idempotent fast path) #461

Description

@gustavdelius

Summary

validParams() (and validSim(), which calls it) is idempotent but not cheap on an already-valid object: it redoes its full work every call even when nothing needs fixing. This makes it costly to apply the "validate/normalise at the boundary" principle consistently — e.g. adding validParams() to the individual setters, or validSim() to the sim-consuming getters/plots that currently skip it. This issue proposes making the idempotent path cheap so consistent use becomes essentially free.

Motivation

Measured on the 12-species NS_params / a short sim:

call median
validParams(p) 5.4 ms
validSim(s) 7.6 ms
getBiomass(s) (reference getter) 0.4 ms
project(t_max=1) (reference) 31 ms

A single interactive call is imperceptible, and the core is safe (the C++ timestep loop and the iterative solvers validate once at the top and use the fast project_simple() path inside). The problem is R-level loops: wrapping getBiomass() in validSim() would turn a 0.4 ms getter into ~8 ms — a ~20x regression that repeated-getter workflows (analysis scripts, downstream packages) would feel. Making the idempotent path cheap removes that objection.

Where the cost is

Breakdown of the 5.4 ms:

step cost nature
validSpeciesParams() 3.20 ms repair — deterministic function of the species table
validObject() (S4 validity) 1.13 ms verify — structural, no mutation
validGivenSpeciesParams() 0.39 ms repair — deterministic function of the given table
ft_mask recompute 0.08 ms repair — function of w_max + w_full
is.finite() array scans (14 arrays) 0.05 ms verify — array values
upgrade checks + w checks + w_min_idx + coerce ~0.5 ms mixed
proposed gate: rlang::hash(small inputs) 0.02 ms

The two whales are validSpeciesParams() and validObject(). validParams() is really doing three separable jobs:

  1. Upgrade old objects — already cheaply gated by mizer_needs_upgrading() (version-string compare). Leave as is.
  2. Repair/normalise — rebuild the species tables, defaults, w_min_idx, ft_mask. A deterministic function of a handful of small inputs (the two species tables, gear table, w/w_full, version). On a valid object it recomputes identical values — 3.6 ms of pure waste.
  3. Verifyis.finite() scans and validObject(). Pure checks.

Design: gate the repair block on a hash of the inputs (not a flag)

A boolean validated flag, or comparing the existing time_modified slot against a stored time_validated, would be unsafe here: AGENTS.md deliberately allows package code to write slots directly (params@species_params <- ...), and only the setters bump time_modified. A direct write would leave such a flag/timestamp stale → validParams() skips the rebuild on a genuinely-changed object → silent corruption.

A hash recomputed from the current slot contents on every call cannot go stale: any mutation to given_species_params by any route changes the hash and the full path runs. The optimisation becomes behaviour-preserving — it only ever skips provably-redundant work. Hashing the small inputs costs 0.02 ms (rlang::hash), ~180x below the 3.6 ms it guards.

Implementation

Add one slot (via the /upgrade-mizer-data class-change workflow; a dedicated slot keeps this out of user-facing metadata, which saveParams/compareParams inspect):

# in setClass("MizerParams", ...) representation
validation_hash = "ANY"      # default NA; the fingerprint at last full validation

Split the monolith and add the fast path:

validation_key <- function(params) {
    rlang::hash(list(params@given_species_params,
                     params@gear_params,
                     params@w, params@w_full,
                     as.character(params@mizer_version),
                     params@extensions))
}

validParams.MizerParams <- function(params, info_level = 3) {
    # 1. Upgrades — unchanged, already cheaply gated
    if (mizer_needs_upgrading(params)) {
        params <- suppressWarnings(upgrade.MizerParams(params))
        if (info_level > 0) warning("Your MizerParams object was created with ...")
    }
    if (extension_needs_upgrading(params)) {
        params <- suppressWarnings(runExtensionUpgrades(params))
    }

    # 2. Repair — skip when the inputs it depends on are unchanged
    if (!identical(validation_key(params), params@validation_hash)) {
        params <- repair_params(params)                    # the current expensive body
        params@validation_hash <- validation_key(params)   # recompute AFTER repair
        validObject(params)                                # structural check, gated with repair
    }

    # 3. Verify array *values* — always, the cheap safety net (0.05 ms)
    check_finite(params)
    coerceToExtensionClass(params)
}

Two correctness subtleties:

  • Recompute the stored hash after repair_params(). validGivenSpeciesParams() can itself modify given_species_params (e.g. filling weight from length). Because that repair is idempotent, storing the post-repair key means the next call recomputes the same key from the unchanged slots and hits the fast path. Storing the pre-repair key would never produce a hit.
  • Keep the is.finite() scans (job 3) unconditional and outside the gate. They're 0.05 ms and catch exactly what the hash can't see: a setter writing NaN into e.g. search_vol without touching the species table. validObject() (structural validity) can move inside the gate, because the dimension invariants it checks are all functions of the hashed inputs (species count, grid length); a change that breaks them changes the hash. (Confirm validMizerParams contains no value-level array checks before gating it.)

Resulting floor

  • Conservative (keep validObject() unconditional): fast path ≈ hash + finite + validObject ≈ ~1.2 ms (from 5.4) — ~4.5x, zero staleness risk.
  • Aggressive (gate validObject() with the repair, as sketched): fast path ≈ hash + finite ≈ ~0.07 ms — ~75x. Array-value corruption still caught unconditionally by check_finite(); structural corruption necessarily changes the fingerprint.

I'd favour the aggressive version.

validSim() falls out for free

validSim()'s 7.6 ms is dominated by its nested validParams(sim@params) call; fixing validParams() drops that to ~0.1 ms automatically. Its only remaining own cost is the is.finite(sim@n) scan, O(time x species x size), which should stay unconditional (it detects a blown-up simulation). Do not hash sim@n — the one big array, whose hash costs as much as the scan. No separate caching needed for validSim.

Caveats

  • The gate helps the fixed data-frame overhead (misspelling regexes, default-setting, table copying) that dominates validSpeciesParams, not anything proportional to array size — but those parts were already cheap.
  • One warm-up per object: the first validParams() after construction or any real change runs the full path and stores the hash (the existing cost, just no longer repeated).
  • saveParams/readParams round-trips carry the stored hash, so a reloaded-but-unchanged model hits the fast path; a mizer version bump invalidates it automatically (version is in the key).
  • Adding the slot pulls in the /upgrade-mizer-data workflow and a NEWS.md entry.

Follow-up (separate issue/PR)

Once the idempotent path is cheap, revisit the two consistency gaps this unblocks: adding validParams() to the individual setters (setFishing(), setMetabolicRate(), ...), and validSim() to the sim-consuming getters/plots (summary_methods.R, plots.R) that currently rely on S3 dispatch alone.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions