From bb2510b18cf3481314ead39f116fa62be3771226 Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Thu, 21 May 2026 11:55:27 -0700 Subject: [PATCH 1/3] core: canonicalize arrayed element keys in projectAttachData Simulation results key each arrayed variable's per-element series by the engine's canonicalized element name (e.g. `temperature[high_2xco2_sensitivity]`), but a Dimension preserves the model's original-case subscript names (`High_2xCO2_sensitivity`). projectAttachData built its lookup keys from the original-case subscripts, so for any arrayed variable whose dimension elements are not already lowercase the keys never matched and the variable received an empty data array -- the diagram then rendered no sparkline for it. This silently blanked every arrayed variable in C-LEARN (its scenario dimension is {Deterministic, Low_2xCO2_sensitivity, High_2xCO2_sensitivity}), which is why the model appeared to produce no results even though the engine simulates it correctly and isSimulatable() is true. Canonicalizing the element when building the key restores per-element data (verified end-to-end: variables with attached data went from 492 to 814 on C-LEARN). Multi-dimensional arrayed variables remain unhandled by the dimNames.length !== 1 guard -- a separate limitation. --- src/core/datamodel.ts | 7 ++- src/core/tests/datamodel.test.ts | 97 ++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 1 deletion(-) diff --git a/src/core/datamodel.ts b/src/core/datamodel.ts index f115bde78..0695e0c6b 100644 --- a/src/core/datamodel.ts +++ b/src/core/datamodel.ts @@ -1481,8 +1481,13 @@ export function projectAttachData(project: Project, data: ReadonlyMap data.get(`${ident}[${element}]`)) + .map((element) => data.get(`${ident}[${canonicalize(element)}]`)) .filter((d) => d !== undefined) .map((d) => defined(d)); diff --git a/src/core/tests/datamodel.test.ts b/src/core/tests/datamodel.test.ts index 64e90f18c..b845fcad7 100644 --- a/src/core/tests/datamodel.test.ts +++ b/src/core/tests/datamodel.test.ts @@ -35,6 +35,7 @@ import { modelToJson, projectFromJson, projectToJson, + projectAttachData, } from '../datamodel'; import type { @@ -68,6 +69,7 @@ import type { JsonLinkViewElement, JsonCloudViewElement, } from '@simlin/engine'; +import { defined, type Series } from '../common'; describe('GraphicalFunctionScale', () => { it('should roundtrip correctly', () => { @@ -1244,3 +1246,98 @@ describe('Project', () => { expect(restored.source).toBeUndefined(); }); }); + +describe('projectAttachData', () => { + const series = (name: string, values: number[]): Series => ({ + name, + time: new Float64Array([0, 1, 2]), + values: new Float64Array(values), + }); + + const arrayedAux = (ident: string, dimensionNames: string[]): Aux => ({ + type: 'aux', + ident, + equation: { type: 'applyToAll', dimensionNames, equation: '1' }, + documentation: '', + units: '', + gf: undefined, + canBeModuleInput: false, + isPublic: false, + data: undefined, + errors: undefined, + unitErrors: undefined, + uid: 1, + }); + + const projectWith = (variables: Variable[], dimensions: Dimension[]): Project => ({ + name: 'test', + simSpecs: { + start: 0, + stop: 2, + dt: { value: 1, isReciprocal: false }, + saveStep: undefined, + simMethod: 'euler', + timeUnits: '', + }, + models: new Map([ + [ + 'main', + { + name: 'main', + variables: new Map(variables.map((v) => [v.ident, v])), + views: [], + loopMetadata: [], + groups: [], + }, + ], + ]), + dimensions: new Map(dimensions.map((d) => [d.name, d])), + hasNoEquations: false, + source: undefined, + }); + + // Regression test for arrayed-variable sparklines: the simulation keys its + // per-element series by CANONICALIZED element names (e.g. + // `temperature[high_2xco2_sensitivity]`), but a dimension preserves the + // model's ORIGINAL-case subscript names. projectAttachData must canonicalize + // each element when building the lookup key, or every arrayed variable whose + // dimension elements aren't already lowercase gets no data. + it('attaches per-element data for a 1-D arrayed variable with original-case subscripts', () => { + const project = projectWith( + [arrayedAux('temperature', ['scenario'])], + [{ name: 'scenario', subscripts: ['Deterministic', 'Low_2xCO2_sensitivity', 'High_2xCO2_sensitivity'] }], + ); + const data = new Map([ + ['temperature[deterministic]', series('temperature[deterministic]', [1, 2, 3])], + ['temperature[low_2xco2_sensitivity]', series('temperature[low_2xco2_sensitivity]', [4, 5, 6])], + ['temperature[high_2xco2_sensitivity]', series('temperature[high_2xco2_sensitivity]', [7, 8, 9])], + ]); + + const attached = projectAttachData(project, data, 'main'); + const v = defined(attached.models.get('main')).variables.get('temperature'); + + expect(v?.data).toBeDefined(); + // ordered by the dimension's declared subscript order + expect((v?.data ?? []).map((s) => Array.from(s.values))).toEqual([ + [1, 2, 3], + [4, 5, 6], + [7, 8, 9], + ]); + }); + + it('attaches data for an already-canonical 1-D arrayed variable', () => { + const project = projectWith( + [arrayedAux('population', ['region'])], + [{ name: 'region', subscripts: ['boston', 'nyc'] }], + ); + const data = new Map([ + ['population[boston]', series('population[boston]', [10, 11])], + ['population[nyc]', series('population[nyc]', [20, 21])], + ]); + + const attached = projectAttachData(project, data, 'main'); + const v = defined(attached.models.get('main')).variables.get('population'); + + expect((v?.data ?? []).length).toBe(2); + }); +}); From 5f3266c90b87cebcb115af3aa83575d03f2577d9 Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Thu, 21 May 2026 12:54:18 -0700 Subject: [PATCH 2/3] core: attach all element series for arrayed vars of any rank Generalizes projectAttachData to group every result series by its base variable ident (the text before the first `[`) and attach all of them. A scalar gets its single bare-keyed series; an arrayed variable gets every per-element series the simulation emitted, for any dimensionality. This supersedes the previous 1-D-only path (which also had to canonicalize element names to match keys) and removes the `dimNames.length !== 1` guard that silently dropped all multi-dimensional arrayed variables. Because grouping matches whatever keys the run produced rather than reconstructing them from a Dimension's original-case subscripts, the element-name canonicalization mismatch can no longer recur. The display layer already plots every series (LineChart in VariableDetails draws one line per element; the canvas Sparkline draws one path per element), so attaching the full set is all that was needed. Verified end-to-end on C-LEARN: variables with attached data went 814 -> 873; a scalar attaches 1 series, a 1-D arrayed var 3, and the multi-dimensional co2eq_emissions_from_hfc all 63. --- src/core/datamodel.ts | 48 ++++++++++++++++---------------- src/core/tests/datamodel.test.ts | 36 ++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 24 deletions(-) diff --git a/src/core/datamodel.ts b/src/core/datamodel.ts index 0695e0c6b..85e126240 100644 --- a/src/core/datamodel.ts +++ b/src/core/datamodel.ts @@ -1464,33 +1464,33 @@ export function projectToJson(project: Project): JsonProject { export function projectAttachData(project: Project, data: ReadonlyMap, modelName: string): Project { const model = defined(project.models.get(modelName)); - const variables = mapValues(model.variables, (v: Variable) => { - if (data.has(v.ident)) { - return { ...v, data: [defined(data.get(v.ident))] }; - } - if (!variableIsArrayed(v)) { - return v; - } - const eqn = variableEquation(v); - if (!eqn || (eqn.type !== 'applyToAll' && eqn.type !== 'arrayed')) { - return v; + + // Group every result series by its base variable ident. A scalar variable's + // series is keyed by the bare canonical ident; an arrayed variable's + // per-element series are keyed `ident[]` for any + // dimensionality (1-D `x[a]`, multi-D `x[a,b]`). Grouping by the ident before + // the first `[` attaches every element series -- so multi-dimensional + // variables are plotted too -- and matches whatever the simulation emitted + // rather than reconstructing keys from a Dimension's (original-case) + // subscripts, which avoids the element-name canonicalization mismatch + // entirely. + const seriesByIdent = new Map(); + for (const [key, s] of data) { + const open = key.indexOf('['); + const ident = open === -1 ? key : key.slice(0, open); + const existing = seriesByIdent.get(ident); + if (existing) { + existing.push(s); + } else { + seriesByIdent.set(ident, [s]); } - const dimNames = eqn.dimensionNames; - if (dimNames.length !== 1) { + } + + const variables = mapValues(model.variables, (v: Variable) => { + const series = seriesByIdent.get(v.ident); + if (!series || series.length === 0) { return v; } - const ident = v.ident; - const dim = defined(project.dimensions.get(dimNames[0])); - // Simulation results key per-element series by canonicalized element names - // (e.g. `temperature[high_2xco2_sensitivity]`), but a dimension preserves - // the model's original-case subscript names (`High_2xCO2_sensitivity`). - // Canonicalize each element to match, otherwise an arrayed variable whose - // elements aren't already lowercase gets no data and renders no sparkline. - const series = dim.subscripts - .map((element) => data.get(`${ident}[${canonicalize(element)}]`)) - .filter((d) => d !== undefined) - .map((d) => defined(d)); - return { ...v, data: series }; }); const updatedModel: Model = { ...model, variables }; diff --git a/src/core/tests/datamodel.test.ts b/src/core/tests/datamodel.test.ts index b845fcad7..e798171ca 100644 --- a/src/core/tests/datamodel.test.ts +++ b/src/core/tests/datamodel.test.ts @@ -1340,4 +1340,40 @@ describe('projectAttachData', () => { expect((v?.data ?? []).length).toBe(2); }); + + // The "plot all the series" behavior must also cover multi-dimensional + // arrayed variables: the simulation emits one series per element of the + // cartesian product (`flux[co2,deterministic]`, ...), and every one should + // be attached so the chart/sparkline can draw them all. + it('attaches all per-element series for a multi-dimensional arrayed variable', () => { + const project = projectWith( + [arrayedAux('flux', ['gas', 'scenario'])], + [ + { name: 'gas', subscripts: ['CO2', 'CH4'] }, + { name: 'scenario', subscripts: ['Deterministic', 'High_2xCO2_sensitivity'] }, + ], + ); + const data = new Map([ + ['flux[co2,deterministic]', series('flux[co2,deterministic]', [1, 1])], + ['flux[co2,high_2xco2_sensitivity]', series('flux[co2,high_2xco2_sensitivity]', [2, 2])], + ['flux[ch4,deterministic]', series('flux[ch4,deterministic]', [3, 3])], + ['flux[ch4,high_2xco2_sensitivity]', series('flux[ch4,high_2xco2_sensitivity]', [4, 4])], + ]); + + const attached = projectAttachData(project, data, 'main'); + const v = defined(attached.models.get('main')).variables.get('flux'); + + expect((v?.data ?? []).length).toBe(4); + }); + + it('leaves a variable with no result series unchanged', () => { + const project = projectWith( + [arrayedAux('unused', ['scenario'])], + [{ name: 'scenario', subscripts: ['Deterministic'] }], + ); + const attached = projectAttachData(project, new Map(), 'main'); + const v = defined(attached.models.get('main')).variables.get('unused'); + + expect(v?.data).toBeUndefined(); + }); }); From f860bfaee933525f4641719cc992f9eec3a4eacc Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Thu, 21 May 2026 13:03:40 -0700 Subject: [PATCH 3/3] diagram: show results chart for variables with unit errors The variable details panel previously replaced the chart with an error list whenever the variable had ANY error, including a non-fatal unit error. A unit error does not stop the variable from simulating, so it has valid data worth charting; hiding the chart meant a model with many unit errors (e.g. C-LEARN, 481) showed almost no results in the details panel. Now only genuine equation/compile errors (which leave the variable with no data) replace the chart; unit errors are surfaced as warnings beneath the chart instead. The chart-vs-errors decision is a small pure helper (variableDetailsView) so it is unit-tested directly. Together with the projectAttachData fix, both scalar (one line) and arrayed (one line per element) variables now display their results in the details chart even when they carry unit warnings. --- src/diagram/VariableDetails.tsx | 50 ++++++++------ .../tests/variable-details-display.test.ts | 65 +++++++++++++++++++ src/diagram/variable-details-display.ts | 34 ++++++++++ 3 files changed, 128 insertions(+), 21 deletions(-) create mode 100644 src/diagram/tests/variable-details-display.test.ts create mode 100644 src/diagram/variable-details-display.ts diff --git a/src/diagram/VariableDetails.tsx b/src/diagram/VariableDetails.tsx index e272921a3..ec58f363d 100644 --- a/src/diagram/VariableDetails.tsx +++ b/src/diagram/VariableDetails.tsx @@ -28,6 +28,7 @@ import { plainDeserialize, plainSerialize } from './drawing/common'; import { CustomElement, FormattedText, CustomEditor } from './drawing/SlateEditor'; import { caretOffsetForClick, caretOffsetWithinSpan, RenderedGlyph } from './equation-caret'; import { LookupEditor } from './LookupEditor'; +import { variableDetailsView } from './variable-details-display'; import { errorCodeDescription } from '@simlin/engine'; import styles from './VariableDetails.module.css'; @@ -386,31 +387,38 @@ export class VariableDetails extends React.PureComponent = []; - if (errors) { - errors.forEach((error) => { - errorList.push(
error: {errorCodeDescription(error.code)}
); - }); - } - if (unitErrors) { - unitErrors.forEach((error) => { - const details = error.details; - errorList.push( -
- unit error: {errorCodeDescription(error.code)} - {details ? `: ${details}` : undefined} -
, - ); - }); - } - chartOrErrors = errorList; + const detailsView = variableDetailsView(this.props.variable); + // Unit errors are non-fatal warnings: the variable still simulates and has + // data. They are rendered beneath the chart (or alongside equation errors) + // rather than replacing the results. + const unitWarnings = detailsView.unitWarnings.map((error, i) => { + const details = error.details; + return ( +
+ unit error: {errorCodeDescription(error.code)} + {details ? `: ${details}` : undefined} +
+ ); + }); + + let chartOrErrors; + if (!detailsView.showChart) { + // Equation/compile errors mean the variable produced no valid data, so + // the error list replaces the chart. + const errorList = detailsView.equationErrors.map((error, i) => ( +
+ error: {errorCodeDescription(error.code)} +
+ )); + chartOrErrors = [...errorList, ...unitWarnings]; } else { chartOrErrors = ( - + <> + + {unitWarnings} + ); } diff --git a/src/diagram/tests/variable-details-display.test.ts b/src/diagram/tests/variable-details-display.test.ts new file mode 100644 index 000000000..551462140 --- /dev/null +++ b/src/diagram/tests/variable-details-display.test.ts @@ -0,0 +1,65 @@ +// Copyright 2026 The Simlin Authors. All rights reserved. +// Use of this source code is governed by the Apache License, +// Version 2.0, that can be found in the LICENSE file. + +import { ErrorCode } from '@simlin/core/datamodel'; +import type { Aux, EquationError, UnitError, Variable } from '@simlin/core/datamodel'; + +import { variableDetailsView } from '../variable-details-display'; + +function aux(overrides: Partial = {}): Variable { + return { + type: 'aux', + ident: 'x', + equation: { type: 'scalar', equation: '1' }, + documentation: '', + units: '', + gf: undefined, + canBeModuleInput: false, + isPublic: false, + data: undefined, + errors: undefined, + unitErrors: undefined, + uid: 1, + ...overrides, + }; +} + +const equationError: EquationError = { code: ErrorCode.EmptyEquation, start: 0, end: 0 }; +const unitError: UnitError = { + code: ErrorCode.BadTable, + start: 0, + end: 0, + isConsistencyError: true, + details: 'dimensions are not equal', +}; + +describe('variableDetailsView', () => { + it('shows the chart when the variable has no errors', () => { + expect(variableDetailsView(aux())).toEqual({ + showChart: true, + equationErrors: [], + unitWarnings: [], + }); + }); + + it('keeps the chart and surfaces unit errors as warnings (not fatal)', () => { + const view = variableDetailsView(aux({ unitErrors: [unitError] })); + expect(view.showChart).toBe(true); + expect(view.unitWarnings).toEqual([unitError]); + expect(view.equationErrors).toEqual([]); + }); + + it('replaces the chart with equation/compile errors (no valid data)', () => { + const view = variableDetailsView(aux({ errors: [equationError] })); + expect(view.showChart).toBe(false); + expect(view.equationErrors).toEqual([equationError]); + }); + + it('prefers equation errors over the chart even when unit errors also exist', () => { + const view = variableDetailsView(aux({ errors: [equationError], unitErrors: [unitError] })); + expect(view.showChart).toBe(false); + expect(view.equationErrors).toEqual([equationError]); + expect(view.unitWarnings).toEqual([unitError]); + }); +}); diff --git a/src/diagram/variable-details-display.ts b/src/diagram/variable-details-display.ts new file mode 100644 index 000000000..cbd2c9373 --- /dev/null +++ b/src/diagram/variable-details-display.ts @@ -0,0 +1,34 @@ +// Copyright 2026 The Simlin Authors. All rights reserved. +// Use of this source code is governed by the Apache License, +// Version 2.0, that can be found in the LICENSE file. + +import type { EquationError, UnitError, Variable } from '@simlin/core/datamodel'; + +export interface VariableDetailsView { + /** + * Whether to render the results chart. False only when the variable has + * equation/compile errors -- those mean it produced no valid data, so the + * error list takes the chart's place. + */ + readonly showChart: boolean; + /** Equation/compile errors; rendered as the error list when showChart is false. */ + readonly equationErrors: readonly EquationError[]; + /** Non-fatal unit errors; surfaced as warnings beside the chart. */ + readonly unitWarnings: readonly UnitError[]; +} + +/** + * Decide what the variable-details panel shows for a variable. Unit errors are + * non-fatal -- the variable still simulates and has data -- so they no longer + * hide the chart; only genuine equation/compile errors (which leave the + * variable with no valid data) replace it. + */ +export function variableDetailsView(variable: Variable): VariableDetailsView { + const equationErrors = variable.errors ?? []; + const unitWarnings = variable.unitErrors ?? []; + return { + showChart: equationErrors.length === 0, + equationErrors, + unitWarnings, + }; +}