Resolve UNRESOLVED.initConc and UNRESOLVED.Size in SBML import

[jcschaff]

Resolves #803 — both halves. initConc by hoisting into a global parameter, Size by using the model-level StructureSize.

The problem

SBML has one flat namespace; VCell separates physiology from application. An SBML global parameter becomes a Model parameter, while a species' initial concentration is a SpeciesContextSpec parameter under the SimulationContext. Those two name scopes are unrelated roots — ModelNameScope.getParent() and SimulationContextNameScope.getParent() both return null, and neither is the other's peer — so a Model parameter cannot name an initial concentration, and getRelativeScopePrefix yields the UNRESOLVED. marker. The import then dies much later with:

Error binding global parameter 'beta' to model: 'UNRESOLVED.initConc' is either not found in your model or is not allowed to be used in the current context.

31 curated BioModels fail this way. #803 has been open since 2023.

The fix: invert the dependency, don't drop it

For SBML beta = c1/(N1*s4) where s4 is a species:

before   s4.initConc = 250000              beta = c1/(N1 * UNRESOLVED.initConc)   [broken]
after    s4_initConc = 250000   (global)   s4.initConc = s4_initConc
                                           beta = c1/(N1 * s4_initConc)           [exact]

Verified on the imported model rather than from the log:

s4_initConc = 250000.0          beta        = (c1 / (N1 * s4_initConc))
s2_initConc = 48000.0           s4.initConc = s4_initConc
                                s2.initConc = s2_initConc

Nothing is lost. The relationship stays symbolic, so scanning s4_initConc moves the initial condition and beta together — which is what the SBML meant, and is the VCell idiom for varying an initial condition. Hoisting happens once per species, so N dependents produce one parameter, not N.

Everything stays a global parameter, which matters: global parameters already round-trip through SBMLExporter, whereas SimulationContextParameter does not (#1984). This fix therefore does not wait on that work.

One implementation detail worth knowing

The reference is written as a plain name, deliberately. A species' initial condition resolves it through SimulationContext.getLocalEntry(), which falls through to getModel().getLocalEntry(). Writing it as new Expression(ste, namescope) would ask the scope machinery for a prefix and get UNRESOLVED. straight back. I probed that both directions bind before building on it.

An earlier attempt, rejected

I first inlined the constant value (beta = 1.539e-7). Rejected on review — it freezes the dependency, so the imported model reads as a magic number and an export no longer reproduces the source.

It was also strictly weaker: it could only act when the initial condition was a literal, so model 632 — whose species initial condition is itself computed — stayed broken. Hoisting handles it because it moves the expression, not the value.

Compartment Size: fixed, not deferred

A compartment size has two representations: the StructureMapping Size parameter, which belongs to the application, and Structure.StructureSize, a ModelQuantity that belongs to the physiology and that ModelNameScope names directly.

adjustExpression already swapped to the latter when the target was a species initial condition — it just didn't when the target was in Model scope. Making that swap unconditional fixes the 7 UNRESOLVED.Size models. No hoisting needed, and the size stays a constant, so StructureSizeSolver, GeometryContext and SBMLExporter — all of which call evaluateConstant() on it — are unaffected.

BIOMD0000000457 now imports. 429 and 1027 still fail, on a compartment with constant="false" — a different unsupported feature this failure was hiding.

No reporting left

An earlier revision reported unresolved references instead of fixing them. Resolving them is better, and with both classes handled no UNRESOLVED. marker survives anywhere in the 27-model import suite — the report was dead code and is gone, along with the collector list that fed the initial-condition message.

What remains is one logger.error inside adjustExpression for a scope mismatch we have not seen, since the eventual binding failure names only the marker and not what produced it. No collector, no call-site plumbing, and it covers all twelve call sites rather than two.

Verification

model	before	after
872	UNRESOLVED.initConc	imports
599, 705, 696	UNRESOLVED.initConc	import
632	UNRESOLVED.initConc	imports — the case inlining could not do
627	UNRESOLVED.initConc	still fails, on a reaction-rate reference (#1983) that was hidden behind this
457	UNRESOLVED.Size	imports
429, 1027	UNRESOLVED.Size	still fail, on constant="false" compartments — a different feature this was hiding

suite	result
SBMLTestSuiteTest — computed results vs reference CSVs, within tolerance	1382 tests, 0 failures
BMDB_SBMLImportTest (SBML_IT)	27 tests, 0 failures — 696 removed from the fault table, it now passes
vcell-core Fast, CI parallel flags	547 tests, 1 error (VCellDataTest poetry noise, environmental)
vcell-math Fast	6545 tests, 0 failures

The first row is the one that matters. "It imports" says nothing about whether the math is still right; the SBML Test Suite compares simulated output against reference results, so it is the check that these scope changes preserve semantics rather than just parseability.

AbstractNameScope gains a named constant for the "UNRESOLVED." literal, so callers that can do better on that path can test for it rather than matching a magic string.

Refs #803, #1984

🤖 Generated with Claude Code