Skip to content

Commit

Permalink
feat: add support for comparison scenarios that have different input …
Browse files Browse the repository at this point in the history
…settings in the two models (#456)

Fixes #453
  • Loading branch information
chrispcampbell authored Mar 26, 2024
1 parent 65560ef commit e250cdf
Show file tree
Hide file tree
Showing 7 changed files with 261 additions and 10 deletions.
11 changes: 11 additions & 0 deletions examples/sample-check-tests/src/comparisons/comparison-specs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,17 @@ export function createBaseComparisonSpecs(bundleL: Bundle, bundleR: Bundle): Com
addScenario(inputId, 'max')
}

// Create a special scenario that controls a different set of inputs in the "left" model
// than in the "right" model
scenarios.push({
kind: 'scenario-with-distinct-inputs',
id: 'extreme_main_sliders_at_best_case',
title: 'Main sliders',
subtitle: 'at best case',
inputsL: [{ kind: 'input-at-value', inputName: 'Input A', value: 75 }],
inputsR: [{ kind: 'input-at-value', inputName: 'Input B prime', value: 25 }]
})

return {
scenarios,
scenarioGroups: [],
Expand Down
8 changes: 8 additions & 0 deletions examples/sample-check-tests/src/comparisons/comparisons.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@
scenario_ref: baseline
graphs: all

- view_group:
title: Extremes
views:
- view:
scenario_ref: extreme_main_sliders_at_best_case
graphs:
- '1'

- view_group:
title: Group 1 (showing graphs 1+2 for different scenarios involving Input 1)
views:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type {
ComparisonScenarioRefSpec,
ComparisonScenarioSpec,
ComparisonScenarioWithAllInputsSpec,
ComparisonScenarioWithDistinctInputsSpec,
ComparisonScenarioWithInputsSpec,
ComparisonSpecs,
ComparisonViewGraphsArraySpec,
Expand Down Expand Up @@ -60,6 +61,21 @@ export function scenarioWithInputsSpec(
}
}

export function scenarioWithDistinctInputsSpec(
inputsL: ComparisonScenarioInputSpec[],
inputsR: ComparisonScenarioInputSpec[],
opts?: { id?: string; title?: string; subtitle?: string }
): ComparisonScenarioWithDistinctInputsSpec {
return {
kind: 'scenario-with-distinct-inputs',
id: opts?.id,
title: opts?.title,
subtitle: opts?.subtitle,
inputsL,
inputsR
}
}

export function inputAtPositionSpec(
inputName: string,
position: ComparisonScenarioInputPosition
Expand Down
19 changes: 19 additions & 0 deletions packages/check-core/src/comparison/config/comparison-spec-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,24 @@ export interface ComparisonScenarioWithInputsSpec {
inputs: ComparisonScenarioInputSpec[]
}

/**
* Specifies a single scenario that configures inputs differently for the two
* model instances.
*/
export interface ComparisonScenarioWithDistinctInputsSpec {
kind: 'scenario-with-distinct-inputs'
/** The unique identifier for the scenario. */
id?: ComparisonScenarioId
/** The title of the scenario. */
title?: ComparisonScenarioTitle
/** The subtitle of the scenario. */
subtitle?: ComparisonScenarioSubtitle
/** The input settings for this scenario when run with the "left" model. */
inputsL: ComparisonScenarioInputSpec[]
/** The input settings for this scenario when run with the "right" model. */
inputsR: ComparisonScenarioInputSpec[]
}

/**
* Specifies a single scenario that sets all available inputs to position.
*/
Expand Down Expand Up @@ -88,6 +106,7 @@ export interface ComparisonScenarioPresetMatrixSpec {
*/
export type ComparisonScenarioSpec =
| ComparisonScenarioWithInputsSpec
| ComparisonScenarioWithDistinctInputsSpec
| ComparisonScenarioWithAllInputsSpec
| ComparisonScenarioPresetMatrixSpec

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,19 @@ import type { InputId, InputVar } from '../../../bundle/var-types'

import type { ComparisonSpecs } from '../comparison-spec-types'

import type { ComparisonScenario, ComparisonScenarioKey } from '../../_shared/comparison-resolved-types'
import type {
ComparisonScenario,
ComparisonScenarioInput,
ComparisonScenarioInputState,
ComparisonScenarioKey
} from '../../_shared/comparison-resolved-types'

import {
allAtPos,
inputVar,
scenarioGroup,
scenarioWithInput,
scenarioWithInputs,
unresolvedScenarioRef,
unresolvedViewForScenarioGroupId,
unresolvedViewForScenarioId,
Expand All @@ -35,6 +41,7 @@ import {
scenarioMatrixSpec,
scenarioRefSpec,
scenarioWithAllInputsSpec,
scenarioWithDistinctInputsSpec,
scenarioWithInputsSpec,
viewGroupWithScenariosSpec,
viewGroupWithViewsSpec,
Expand Down Expand Up @@ -244,6 +251,67 @@ describe('resolveComparisonSpecs', () => {
})
})

it('should expand distinct model-specific input specs', () => {
function resolvedInput(
requestedInputName: string,
stateL: ComparisonScenarioInputState,
stateR: ComparisonScenarioInputState
): ComparisonScenarioInput {
return {
requestedName: requestedInputName,
stateL,
stateR
}
}

const specs = comparisonSpecs([
// Match by variable name
scenarioWithDistinctInputsSpec([inputAtValueSpec('ivarA', 20)], [inputAtValueSpec('ivarA', 30)]),
// Match by input ID
scenarioWithDistinctInputsSpec([inputAtValueSpec('id 2', 40)], [inputAtValueSpec('id 2', 50)]),
// Match by alias (slider name)
scenarioWithDistinctInputsSpec([inputAtValueSpec('S3', 60)], [inputAtValueSpec('S3', 70)]),
// Error if input is not available on requested side
scenarioWithDistinctInputsSpec([inputAtValueSpec('ivarA', 20)], [inputAtValueSpec('unknown', 600)]),
// Error if value is out of range on both sides
scenarioWithDistinctInputsSpec([inputAtValueSpec('ivarA', 500)], [inputAtValueSpec('ivarA', 600)]),
// Error if value is out of range on one side
scenarioWithDistinctInputsSpec([inputAtValueSpec('id 2', 90)], [inputAtValueSpec('id 2', 600)])
])

const resolved = resolveComparisonSpecs(modelInputsL, modelInputsR, specs)
expect(resolved).toEqual({
scenarios: [
scenarioWithInputs('1', [], atValSpec('_ivara', 20), atValSpec('_ivara', 30)),
scenarioWithInputs('2', [], atValSpec('_ivarb', 40), atValSpec('_ivarb_renamed', 50)),
scenarioWithInputs('3', [], atValSpec('_ivarc', 60), atValSpec('_ivard', 70)),
scenarioWithInputs(
'4',
[resolvedInput('unknown', {}, { error: { kind: 'unknown-input' } })],
undefined,
undefined
),
scenarioWithInputs(
'5',
[
resolvedInput('ivarA', { error: { kind: 'invalid-value' } }, {}),
resolvedInput('ivarA', {}, { error: { kind: 'invalid-value' } })
],
undefined,
undefined
),
scenarioWithInputs(
'6',
[resolvedInput('id 2', {}, { error: { kind: 'invalid-value' } })],
undefined,
undefined
)
],
scenarioGroups: [],
viewGroups: []
})
})

// it('should expand "with: [...]" multiple setting specs', () => {
// const specs: ComparisonScenarioSpec[] = [
// {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

import { assertNever } from 'assert-never'

import type { InputPosition } from '../../../_shared/scenario-spec-types'
import type { InputPosition, InputSetting, ScenarioSpec } from '../../../_shared/scenario-spec-types'
import { inputSettingsSpec } from '../../../_shared/scenario-specs'

import type { ModelInputs } from '../../../bundle/model-inputs'
import type { InputId, InputVar } from '../../../bundle/var-types'
Expand Down Expand Up @@ -38,7 +39,7 @@ import type {
ComparisonViewSubtitle,
ComparisonViewTitle
} from '../comparison-spec-types'
import { scenarioSpecsFromSettings } from './comparison-scenario-specs'
import { inputSettingFromResolvedInputState, scenarioSpecsFromSettings } from './comparison-scenario-specs'

export interface ComparisonResolvedDefs {
/** The set of resolved scenarios. */
Expand Down Expand Up @@ -236,6 +237,23 @@ function resolveScenariosFromSpec(
]
}

case 'scenario-with-distinct-inputs': {
// Create one scenario in which the inputs are configured differently
// for the two models
return [
resolveScenarioForDistinctInputSpecs(
modelInputsL,
modelInputsR,
genKey(),
scenarioSpec.id,
scenarioSpec.title,
scenarioSpec.subtitle,
scenarioSpec.inputsL,
scenarioSpec.inputsR
)
]
}

default:
assertNever(scenarioSpec)
}
Expand Down Expand Up @@ -361,6 +379,101 @@ function resolveScenarioForInputSpecs(
}
}

/**
* Return a resolved `ComparisonScenario` for the given settings that are different
* for the two models.
*/
function resolveScenarioForDistinctInputSpecs(
modelInputsL: ModelInputs,
modelInputsR: ModelInputs,
key: ComparisonScenarioKey,
id: ComparisonScenarioId | undefined,
title: ComparisonScenarioTitle | undefined,
subtitle: ComparisonScenarioSubtitle | undefined,
inputSpecsL: ComparisonScenarioInputSpec[],
inputSpecsR: ComparisonScenarioInputSpec[]
): ComparisonScenario {
// TODO: Unlike the more typical "scenario with inputs" case, when we have "distinct"
// inputs (separate sets of inputs for the two models) we only include the `settings`
// array in the resulting `ComparisonScenario` if there are errors in resolving the
// inputs. If all inputs were resolved successfully, then the `settings` array will
// be empty. This is probably fine for now but it could stand to be redesigned.
const inputsWithErrors: ComparisonScenarioInput[] = []

// Resolve the input settings for the left and right sides separately
const settingsL: InputSetting[] = []
const settingsR: InputSetting[] = []

// Helper function that resolves an input for the given model/side. If the input is
// resolved successfully, an `InputSetting` will be saved for that side. Otherwise,
// a `ComparisonScenarioInput` describing the error will be saved.
function resolveInputSpec(
side: 'left' | 'right',
modelInputs: ModelInputs,
inputSpec: ComparisonScenarioInputSpec
): void {
let inputState: ComparisonScenarioInputState
switch (inputSpec.kind) {
case 'input-at-position':
inputState = resolveInputForNameInModel(modelInputs, inputSpec.inputName, inputSpec.position)
break
case 'input-at-value':
inputState = resolveInputForNameInModel(modelInputs, inputSpec.inputName, inputSpec.value)
break
default:
assertNever(inputSpec)
}

if (inputState.error !== undefined) {
// The input could not be resolved, so add it to the set of error inputs
// TODO: For now we include an empty object (with undefined properties) for the
// "other" side. Maybe we can make the state properties optional, or maybe we
// just need a less awkward way of handling these input states in the "distinct"
// inputs case.
inputsWithErrors.push({
requestedName: inputSpec.inputName,
stateL: side === 'left' ? inputState : {},
stateR: side === 'right' ? inputState : {}
})
} else {
// The input was resolved, so create a scenario that works for this side
const inputSetting = inputSettingFromResolvedInputState(inputState)
if (side === 'left') {
settingsL.push(inputSetting)
} else {
settingsR.push(inputSetting)
}
}
}

// Resolve the input settings for the left and right sides separately
inputSpecsL.forEach(inputSpec => resolveInputSpec('left', modelInputsL, inputSpec))
inputSpecsR.forEach(inputSpec => resolveInputSpec('right', modelInputsR, inputSpec))

// Create a `ScenarioSpec` for each side if there were no errors
let specL: ScenarioSpec
let specR: ScenarioSpec
if (inputsWithErrors.length === 0) {
specL = inputSettingsSpec(settingsL)
specR = inputSettingsSpec(settingsR)
}

// Create a `ComparisonScenario` with the resolved inputs
return {
kind: 'scenario',
key,
id,
title,
subtitle,
settings: {
kind: 'input-settings',
inputs: inputsWithErrors
},
specL,
specR
}
}

/**
* Return a resolved `ComparisonScenarioInput` for the given input name and position/value.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ import {
valueSetting
} from '../../../_shared/scenario-specs'

import type { ComparisonScenarioInput, ComparisonScenarioSettings } from '../../_shared/comparison-resolved-types'
import type {
ComparisonScenarioInput,
ComparisonScenarioInputState,
ComparisonScenarioSettings
} from '../../_shared/comparison-resolved-types'

/**
* Create a pair of `ScenarioSpec` instances that can be used to run each model given a
Expand All @@ -37,6 +41,10 @@ export function scenarioSpecsFromSettings(
}
}

/**
* Create a `ScenarioSpec` instance that can be used to run a specific model for the
* given input settings.
*/
function scenarioSpecFromInputs(inputs: ComparisonScenarioInput[], side: 'left' | 'right'): ScenarioSpec | undefined {
const settings: InputSetting[] = []

Expand All @@ -51,13 +59,21 @@ function scenarioSpecFromInputs(inputs: ComparisonScenarioInput[], side: 'left'
}

// Create a scenario setting for this input
const varId = state.inputVar.varId
if (state.position) {
settings.push(positionSetting(varId, state.position))
} else {
settings.push(valueSetting(varId, state.value as number))
}
settings.push(inputSettingFromResolvedInputState(state))
}

return inputSettingsSpec(settings)
}

/**
* Create an `InputSetting` instance for the given resolved input that can be used to
* build a `ScenarioSpec`.
*/
export function inputSettingFromResolvedInputState(state: ComparisonScenarioInputState): InputSetting {
const varId = state.inputVar.varId
if (state.position) {
return positionSetting(varId, state.position)
} else {
return valueSetting(varId, state.value as number)
}
}

0 comments on commit e250cdf

Please sign in to comment.