diff --git a/.changeset/clean-mails-accept.md b/.changeset/clean-mails-accept.md new file mode 100644 index 00000000..fbbd193a --- /dev/null +++ b/.changeset/clean-mails-accept.md @@ -0,0 +1,6 @@ +--- +'@boostv/process-optimizer-frontend-core': patch +'@boostv/process-optimizer-frontend-ui': patch +--- + +Improve calculation of the number of experiments to suggest diff --git a/packages/core/src/context/experiment/api.ts b/packages/core/src/context/experiment/api.ts index 1689b0b1..1a163cd5 100644 --- a/packages/core/src/context/experiment/api.ts +++ b/packages/core/src/context/experiment/api.ts @@ -9,6 +9,7 @@ import { calculateSpace, experimentResultSchema, } from '@core/common' +import { selectCalculatedSuggestionCountFromExperiment } from './experiment-selectors' export const createFetchExperimentResultRequest = ( experiment: ExperimentType @@ -25,7 +26,11 @@ export const createFetchExperimentResultRequest = ( experiment.scoreVariables, experiment.dataPoints ), - extras: extras, + extras: { + ...extras, + experimentSuggestionCount: + selectCalculatedSuggestionCountFromExperiment(experiment), + }, optimizerConfig: { acqFunc: cfg.acqFunc, baseEstimator: cfg.baseEstimator, diff --git a/packages/core/src/context/experiment/experiment-reducers.ts b/packages/core/src/context/experiment/experiment-reducers.ts index 55a1cbf1..084f50f8 100644 --- a/packages/core/src/context/experiment/experiment-reducers.ts +++ b/packages/core/src/context/experiment/experiment-reducers.ts @@ -63,17 +63,6 @@ const calculateXi = (state: ExperimentType) => { ) } -const calculateExperimentSuggestionCount = (state: ExperimentType) => { - const dataPoints = state.dataPoints.filter( - d => d.meta.enabled && d.meta.valid - ).length - const initialPoints = state.optimizerConfig.initialPoints - if (dataPoints >= initialPoints) { - return 1 - } - return initialPoints -} - export type ExperimentAction = | { type: 'setSwVersion' @@ -371,8 +360,6 @@ export const experimentReducer = produce( state.optimizerConfig = experimentSchema.shape.optimizerConfig.parse( action.payload ) - state.extras.experimentSuggestionCount = - calculateExperimentSuggestionCount(state) break case 'registerResult': state.lastEvaluationHash = md5( @@ -388,8 +375,6 @@ export const experimentReducer = produce( state.scoreVariables, action.payload ) - state.extras.experimentSuggestionCount = - calculateExperimentSuggestionCount(state) state.optimizerConfig.xi = calculateXi(state) break case 'experiment/toggleMultiObjective': @@ -441,8 +426,6 @@ export const experimentReducer = produce( dimensions: [action.payload], }) } - state.extras.experimentSuggestionCount = - calculateExperimentSuggestionCount(state) break } case 'experiment/removeVariableFromConstraintSum': { @@ -452,8 +435,7 @@ export const experimentReducer = produce( d => d !== action.payload ) } - state.extras.experimentSuggestionCount = - calculateExperimentSuggestionCount(state) + break } default: diff --git a/packages/core/src/context/experiment/experiment-selectors.test.ts b/packages/core/src/context/experiment/experiment-selectors.test.ts index f1d1ed97..6a18477e 100644 --- a/packages/core/src/context/experiment/experiment-selectors.test.ts +++ b/packages/core/src/context/experiment/experiment-selectors.test.ts @@ -1,10 +1,15 @@ import { initialState, State } from '@core/context/experiment/store' import { + selectCalculatedSuggestionCount, selectId, + selectIsConstraintActive, selectIsInitializing, selectIsMultiObjective, + selectIsSuggestionCountEditable, } from './experiment-selectors' import { rootReducer } from './reducers' +import { createDataPoints } from './test-utils' +import { ExperimentType } from '@core/common' describe('Experiment selectors', () => { let state: State @@ -59,4 +64,115 @@ describe('Experiment selectors', () => { expect(after2ndToggle).toEqual(before) }) }) + + describe('selectIsSuggestionCountEditable', () => { + const constraints: ExperimentType['constraints'] = [ + { + type: 'sum', + dimensions: ['a', 'b'], + value: 100, + }, + ] + it.each([ + //data points < initial points, constraint active + [1, 2, constraints, true], + //data points < initial points, constraint NOT active + [1, 2, [], true], + //data points = initial points, constraint active + [1, 1, constraints, false], + //data points = initial points, constraint NOT active + [1, 1, [], true], + //data points > initial points, constraint active + [2, 1, constraints, false], + //data points > initial points, constraint NOT active + [2, 1, [], true], + ])( + 'should be true when data points < intial points or constraints are active', + (dataPoints, initialPoints, constraints, result) => { + const editable = selectIsSuggestionCountEditable({ + ...initialState, + experiment: { + ...initialState.experiment, + dataPoints: createDataPoints(dataPoints), + optimizerConfig: { + ...initialState.experiment.optimizerConfig, + initialPoints, + }, + constraints, + }, + }) + expect(editable).toBe(result) + } + ) + }) + + describe('selectCalculatedSuggestionCount', () => { + const suggestionCount = 5 + const constraints: ExperimentType['constraints'] = [ + { + type: 'sum', + dimensions: ['a', 'b'], + value: 100, + }, + ] + it.each([ + //data points < initial points, constraint active -> initial points + [1, 2, constraints, 2], + //data points < initial points, constraint NOT active -> initial points + [1, 2, [], 2], + //data points = initial points, constraint active -> 1 + [1, 1, constraints, 1], + //data points = initial points, constraint NOT active -> suggestionCount + [1, 1, [], suggestionCount], + //data points > initial points, constraint active -> 1 + [2, 1, constraints, 1], + //data points > initial points, constraint NOT active -> suggestionCount + [2, 1, [], suggestionCount], + ])( + 'should return correct value', + (dataPoints, initialPoints, constraints, result) => { + const editable = selectCalculatedSuggestionCount({ + ...initialState, + experiment: { + ...initialState.experiment, + dataPoints: createDataPoints(dataPoints), + optimizerConfig: { + ...initialState.experiment.optimizerConfig, + initialPoints, + }, + constraints, + extras: { + ...initialState.experiment.extras, + experimentSuggestionCount: suggestionCount, + }, + }, + }) + expect(editable).toBe(result) + } + ) + }) + + describe('selectIsConstraintActive', () => { + it.each([ + [['a', 'b', 'c'], true], + [['a', 'b'], true], + [['a'], false], + [[], false], + ])( + 'should return true when number of sum constraint variables > 1', + (dimensions, result) => { + const editable = selectIsConstraintActive({ + ...initialState.experiment, + constraints: [ + { + type: 'sum', + value: 100, + dimensions, + }, + ], + }) + expect(editable).toBe(result) + } + ) + }) }) diff --git a/packages/core/src/context/experiment/experiment-selectors.ts b/packages/core/src/context/experiment/experiment-selectors.ts index 63f1da46..6479e871 100644 --- a/packages/core/src/context/experiment/experiment-selectors.ts +++ b/packages/core/src/context/experiment/experiment-selectors.ts @@ -13,7 +13,11 @@ export const selectDataPoints = (state: State) => selectExperiment(state).dataPoints export const selectActiveDataPoints = (state: State) => - selectExperiment(state).dataPoints.filter(d => d.meta.valid && d.meta.enabled) + selectActiveDataPointsFromExperiment(selectExperiment(state)) + +export const selectActiveDataPointsFromExperiment = ( + experiment: ExperimentType +) => experiment.dataPoints.filter(d => d.meta.valid && d.meta.enabled) export const selectExpectedMinimum = (state: State) => selectExperiment(state).results.expectedMinimum @@ -51,7 +55,50 @@ export const selectActiveVariableNames = (state: State): string[] => { ) } -export const selectSumConstraint = (state: State) => { - const experiment = selectExperiment(state) - return experiment.constraints.find(c => c.type === 'sum') +export const selectSumConstraint = (state: State) => + selectSumConstraintFromExperiment(selectExperiment(state)) + +export const selectSumConstraintFromExperiment = (experiment: ExperimentType) => + experiment.constraints.find(c => c.type === 'sum') + +export const selectIsConstraintActive = (experiment: ExperimentType) => + (selectSumConstraintFromExperiment(experiment)?.dimensions.length ?? 0) > 1 + +export const selectInitialPoints = (state: State) => + selectInitialPointsFromExperiment(selectExperiment(state)) + +export const selectInitialPointsFromExperiment = (experiment: ExperimentType) => + experiment.optimizerConfig.initialPoints + +export const selectIsSuggestionCountEditable = (state: State) => { + const dataPoints = selectActiveDataPoints(state) + const initialPoints = selectInitialPoints(state) + return ( + dataPoints.length < initialPoints || + !selectIsConstraintActive(selectExperiment(state)) + ) +} + +export const selectSuggestionCountFromExperiment = ( + experiment: ExperimentType +) => + Number.isInteger(experiment.extras['experimentSuggestionCount']) + ? Number(experiment.extras['experimentSuggestionCount']) + : 1 + +export const selectCalculatedSuggestionCount = (state: State) => + selectCalculatedSuggestionCountFromExperiment(selectExperiment(state)) + +export const selectCalculatedSuggestionCountFromExperiment = ( + experiment: ExperimentType +) => { + const dataPoints = selectActiveDataPointsFromExperiment(experiment).length + const initialPoints = selectInitialPointsFromExperiment(experiment) + + if (dataPoints < initialPoints) { + return initialPoints + } else if (selectIsConstraintActive(experiment)) { + return 1 + } + return selectSuggestionCountFromExperiment(experiment) } diff --git a/packages/core/src/context/experiment/reducers.test.ts b/packages/core/src/context/experiment/reducers.test.ts index e7468c3b..691750e0 100644 --- a/packages/core/src/context/experiment/reducers.test.ts +++ b/packages/core/src/context/experiment/reducers.test.ts @@ -250,33 +250,9 @@ describe('experiment reducer', () => { }).experiment.constraints.find(c => c.type === 'sum') expect(actual?.dimensions).toContain('name') }) - - it('should set suggestion count to 1 when adding variable if data points >= initialPoints', () => { - const actual = rootReducer( - { - ...initState, - experiment: { - ...initState.experiment, - optimizerConfig: { - ...initState.experiment.optimizerConfig, - initialPoints: 1, - }, - extras: { - ...initState.experiment.extras, - experimentSuggestionCount: 7, - }, - }, - }, - { - type: 'experiment/addVariableToConstraintSum', - payload: 'name', - } - ) - expect(actual.experiment.extras.experimentSuggestionCount).toBe(1) - }) }) - describe('removeVariableToConstraintSum', () => { + describe('removeVariableFromConstraintSum', () => { it('should remove variable from dimension of constraint', () => { const stateWithConstraint = rootReducer(initState, { type: 'experiment/addVariableToConstraintSum', @@ -288,30 +264,6 @@ describe('experiment reducer', () => { }).experiment.constraints.find(c => c.type === 'sum') expect(actual?.dimensions).not.toContain('name') }) - - it('should set suggestion count to 1 when removing variable if data points >= initialPoints', () => { - const actual = rootReducer( - { - ...initState, - experiment: { - ...initState.experiment, - optimizerConfig: { - ...initState.experiment.optimizerConfig, - initialPoints: 1, - }, - extras: { - ...initState.experiment.extras, - experimentSuggestionCount: 7, - }, - }, - }, - { - type: 'experiment/removeVariableFromConstraintSum', - payload: 'name', - } - ) - expect(actual.experiment.extras.experimentSuggestionCount).toBe(1) - }) }) }) @@ -729,18 +681,6 @@ describe('experiment reducer', () => { expect(actual.optimizerConfig).toMatchObject(payload) }) - it('should set suggested experiments to intial points if length of datapoints is less than initial points', () => { - const payload: OptimizerConfig = { - ...emptyExperiment.optimizerConfig, - initialPoints: 4, - } - const actual = rootReducer(initState, { - type: 'updateConfiguration', - payload, - }).experiment - expect(actual.extras.experimentSuggestionCount).toEqual(4) - }) - it('should not change suggested experiments when changing initial points to something less than length of data points', () => { const payload: OptimizerConfig = { ...emptyExperiment.optimizerConfig, @@ -796,35 +736,6 @@ describe('experiment reducer', () => { expect(actual.dataPoints).toEqual(payload) }) - it('should set suggested experiments to 1 when adding the nth data point where n = initial points', () => { - const testState = produce(initState, draft => { - draft.experiment.extras = { experimentSuggestionCount: 3 } - }) - - const action: ExperimentAction = { - type: 'updateDataPoints', - payload: createDataPoints(3), - } - const actual = rootReducer(testState, action).experiment - expect(actual.extras.experimentSuggestionCount).toEqual(1) - }) - - it('should set suggested experiments to initial points when removing the nth data point where n = initial points', () => { - const testState = produce(initState, draft => { - draft.experiment.extras = { experimentSuggestionCount: 1 } - draft.experiment.dataPoints = createDataPoints(3) - }) - const action: ExperimentAction = { - type: 'updateDataPoints', - payload: createDataPoints(2), - } - - expect( - rootReducer(testState, action).experiment.extras - .experimentSuggestionCount - ).toEqual(3) - }) - it.each(new Array(100).fill(0))( 'should sort data points according to variable definitions', () => { diff --git a/packages/ui/src/containers/result-data/experimentation-guide.tsx b/packages/ui/src/containers/result-data/experimentation-guide.tsx index e5b2d93a..0e95af5c 100644 --- a/packages/ui/src/containers/result-data/experimentation-guide.tsx +++ b/packages/ui/src/containers/result-data/experimentation-guide.tsx @@ -169,7 +169,6 @@ export const ExperimentationGuide = (props: ResultDataProps) => { {!isInitializing && ( dispatchExperiment({ type: 'updateSuggestionCount', diff --git a/packages/ui/src/features/result-data/next-experiments.tsx b/packages/ui/src/features/result-data/next-experiments.tsx index 81b9caa9..8a9b191a 100644 --- a/packages/ui/src/features/result-data/next-experiments.tsx +++ b/packages/ui/src/features/result-data/next-experiments.tsx @@ -1,30 +1,18 @@ import { TextField, Tooltip } from '@mui/material' import { ChangeEvent, FC } from 'react' import { - ExperimentType, - selectActiveDataPoints, - selectSumConstraint, + selectIsSuggestionCountEditable, + selectCalculatedSuggestionCount, useSelector, } from '@boostv/process-optimizer-frontend-core' type Props = { - experiment: ExperimentType onSuggestionChange: (suggestionCount: string) => void } -export const NextExperiments: FC = ({ - experiment, - onSuggestionChange, -}) => { - const sumConstraint = useSelector(selectSumConstraint) - const constraints = sumConstraint?.dimensions.length ?? 0 - const dataPoints = useSelector(selectActiveDataPoints) - const initialPoints = experiment.optimizerConfig.initialPoints - const isSuggestionCountDisabled = - dataPoints.length >= initialPoints && constraints > 1 - - const suggestionCount: number = - (experiment.extras['experimentSuggestionCount'] as number) ?? 1 +export const NextExperiments: FC = ({ onSuggestionChange }) => { + const isSuggestionCountEditable = useSelector(selectIsSuggestionCountEditable) + const suggestionCount = useSelector(selectCalculatedSuggestionCount) const handleSuggestionChange = ( e: ChangeEvent @@ -33,9 +21,9 @@ export const NextExperiments: FC = ({ return ( @@ -46,7 +34,7 @@ export const NextExperiments: FC = ({ label="Suggestions" size="small" onChange={handleSuggestionChange} - disabled={isSuggestionCountDisabled} + disabled={!isSuggestionCountEditable} /> )