Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mrc-4588 Display model fit error details #199

Merged
merged 4 commits into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions app/static/src/app/components/fit/FitTab.vue
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
<div v-if="cancelled" id="fit-cancelled-msg" class="small text-danger">{{cancelledMsg}}</div>
</div>
</fit-plot>
<error-info :error="error"></error-info>
</div>
</div>
</template>
Expand All @@ -39,10 +40,12 @@ import { ModelFitMutation } from "../../store/modelFit/mutations";
import { fitRequirementsExplanation, fitUpdateRequiredExplanation } from "./support";
import { allTrue, anyTrue } from "../../utils";
import LoadingButton from "../LoadingButton.vue";
import ErrorInfo from "../ErrorInfo.vue";

export default defineComponent({
name: "FitTab",
components: {
ErrorInfo,
LoadingSpinner,
FitPlot,
ActionRequiredMessage,
Expand All @@ -57,6 +60,7 @@ export default defineComponent({
const canFitModel = computed(() => allTrue(fitRequirements.value));
const compileRequired = computed(() => store.state.model.compileRequired);
const fitUpdateRequired = computed(() => store.state.modelFit.fitUpdateRequired);
const error = computed(() => store.state.modelFit.error);
const fitModel = () => store.dispatch(`${namespace}/${ModelFitAction.FitModel}`);

const cancelFit = () => store.commit(`${namespace}/${ModelFitMutation.SetFitting}`, false);
Expand All @@ -68,6 +72,10 @@ export default defineComponent({
const sumOfSquares = computed(() => store.state.modelFit.sumOfSquares);

const actionRequiredMessage = computed(() => {
if (error.value) {
return userMessages.modelFit.errorOccurred;
}

if (!allTrue(fitRequirements.value)) {
return fitRequirementsExplanation(fitRequirements.value);
}
Expand Down Expand Up @@ -119,6 +127,7 @@ export default defineComponent({
cancelledMsg,
sumOfSquares,
actionRequiredMessage,
error,
iconType,
iconClass
};
Expand Down
3 changes: 2 additions & 1 deletion app/static/src/app/serialise.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ function serialiseModelFit(modelFit: ModelFitState): SerialisedModelFitState {
converged: modelFit.converged,
sumOfSquares: modelFit.sumOfSquares,
paramsToVary: modelFit.paramsToVary,
result: serialiseSolutionResult(modelFit.result)
result: serialiseSolutionResult(modelFit.result),
error: modelFit.error
};
}

Expand Down
3 changes: 2 additions & 1 deletion app/static/src/app/store/appState/mutations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ export enum AppStateMutation {
export const StateUploadMutations = [
AppStateMutation.ClearQueuedStateUpload,
AppStateMutation.SetQueuedStateUpload,
AppStateMutation.SetStateUploadInProgress
AppStateMutation.SetStateUploadInProgress,
AppStateMutation.SetPersisted
] as string[];

export const appStateMutations: MutationTree<AppState> = {
Expand Down
40 changes: 22 additions & 18 deletions app/static/src/app/store/modelFit/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,24 +50,28 @@ export const actions: ActionTree<ModelFitState, FitState> = {
const { advancedSettings } = rootState.run;
const advancedSettingsOdin = convertAdvancedSettingsToOdin(advancedSettings, pars.base);

const simplex = odinRunnerOde!.wodinFit(
odin!,
data,
pars,
linkedVariable,
advancedSettingsOdin,
{}
);

const inputs = {
data,
endTime,
link
};

commit(ModelFitMutation.SetFitUpdateRequired, null);
commit(ModelFitMutation.SetInputs, inputs);
dispatch(ModelFitAction.FitModelStep, simplex);
try {
const simplex = odinRunnerOde!.wodinFit(
odin!,
data,
pars,
linkedVariable,
advancedSettingsOdin,
{}
);

const inputs = {
data,
endTime,
link
};
commit(ModelFitMutation.SetFitUpdateRequired, null);
commit(ModelFitMutation.SetInputs, inputs);
dispatch(ModelFitAction.FitModelStep, simplex);
} catch (e: unknown) {
commit(ModelFitMutation.SetError, { error: "Model fit error", detail: (e as Error).message });
commit(ModelFitMutation.SetFitting, false);
}
}
},

Expand Down
3 changes: 2 additions & 1 deletion app/static/src/app/store/modelFit/modelFit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ export const defaultState: ModelFitState = {
sumOfSquares: null,
paramsToVary: [],
inputs: null,
result: null
result: null,
error: null
};

export const modelFit = {
Expand Down
12 changes: 10 additions & 2 deletions app/static/src/app/store/modelFit/mutations.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,23 @@
import { MutationTree } from "vuex";
import { ModelFitInputs, ModelFitState, FitUpdateRequiredReasons } from "./state";
import { SimplexResult } from "../../types/responseTypes";
import { SimplexResult, WodinError } from "../../types/responseTypes";

export enum ModelFitMutation {
SetFitting = "SetFitting",
SetResult = "SetResult",
SetInputs = "SetInputs",
SetParamsToVary = "SetParamsToVary",
SetSumOfSquares = "SetSumOfSquares",
SetFitUpdateRequired = "SetFitUpdateRequired"
SetFitUpdateRequired = "SetFitUpdateRequired",
SetError = "SetError"
}

export const mutations: MutationTree<ModelFitState> = {
[ModelFitMutation.SetFitting](state: ModelFitState, payload: boolean) {
state.fitting = payload;
if (payload) {
state.error = null;
}
},

[ModelFitMutation.SetResult](state: ModelFitState, payload: SimplexResult) {
Expand Down Expand Up @@ -43,6 +47,10 @@ export const mutations: MutationTree<ModelFitState> = {
state.sumOfSquares = payload;
},

[ModelFitMutation.SetError](state: ModelFitState, payload: WodinError) {
state.error = payload;
},

[ModelFitMutation.SetFitUpdateRequired](state: ModelFitState, payload: null | Partial<FitUpdateRequiredReasons>) {
if (payload === null) {
state.fitUpdateRequired = {
Expand Down
2 changes: 2 additions & 0 deletions app/static/src/app/store/modelFit/state.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { OdinFitResult } from "../../types/wrapperTypes";
import type { FitData, FitDataLink } from "../fitData/state";
import { WodinError } from "../../types/responseTypes";

export interface ModelFitInputs {
data: FitData;
Expand All @@ -25,6 +26,7 @@ export interface ModelFitState {
paramsToVary: string[],
inputs: ModelFitInputs | null, // all inputs except parameters, which vary
result: OdinFitResult | null, // full solution for current best fit,
error: null | WodinError
}

export interface ModelFitRequirements {
Expand Down
3 changes: 2 additions & 1 deletion app/static/src/app/types/serialisationTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ export interface SerialisedModelFitState {
converged: boolean | null,
sumOfSquares: number | null,
paramsToVary: string[],
result: SerialisedRunResult | null
result: SerialisedRunResult | null,
error: null | WodinError
}

export interface SerialisedAppState {
Expand Down
1 change: 1 addition & 0 deletions app/static/src/app/userMessages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export default {
notFittedYet: "Model has not been fitted.",
selectParamToVary: "Please select at least one parameter to vary during model fit.",
compileRequired: "Model code has been updated. Compile code and Fit Model for updated best fit.",
errorOccurred: "An error occurred during model fit.",
updateFitReasons: {
prefix: "Fit is out of date:",
unknown: "unknown reasons, contact the administrator, as this is unexpected",
Expand Down
35 changes: 35 additions & 0 deletions app/static/tests/e2e/fit.etest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,41 @@ test.describe("Wodin App model fit tests", () => {
expect(newSumOfSquares).not.toEqual(sumOfSquares);
});

test("can show model fit error", async ({ page }) => {
// Upload data
await uploadCSVData(page, realisticFitData);
await page.click(":nth-match(.wodin-right .nav-tabs a, 2)");

// link variables
await page.click(":nth-match(.wodin-left .nav-tabs a, 3)");
const linkContainer = await page.locator(":nth-match(.collapse .container, 1)");
const select1 = await linkContainer.locator(":nth-match(select, 1)");
await select1.selectOption("E");

await expect(await page.innerText("#optimisation label#target-fit-label")).toBe("Cases ~ E");

// select param to vary
await page.click(":nth-match(.wodin-right .nav-tabs a, 2)");
await expect(await page.innerText("#select-param-msg"))
.toBe("Please select at least one parameter to vary during model fit.");
await page.click(":nth-match(input.vary-param-check, 1)");
await page.click(":nth-match(input.vary-param-check, 2)");

// change advanced setting to sabotage fit
await page.click(":nth-match(.collapse-title, 6)"); // Open Advanced Settings
const advancedSettingPanel = await page.locator("#advanced-settings-panel");
const input1 = await advancedSettingPanel.locator(":nth-match(input, 1)");
const input2 = await advancedSettingPanel.locator(":nth-match(input, 2)");
await input1.fill("7");
await input2.fill("-1");

await page.click(".wodin-right .wodin-content div.mt-4 button#fit-btn");

await expect(await page.locator(".fit-tab .action-required-msg"))
.toHaveText("An error occurred during model fit.", { timeout });
await expect(await page.innerText(".fit-tab #error-info")).toContain("Model fit error: Integration failure");
});

test("can see expected update required messages when code changes", async ({ page }) => {
await runFit(page);
await expectUpdateFitMsg(page, "");
Expand Down
1 change: 1 addition & 0 deletions app/static/tests/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ export const mockModelFitState = (state: Partial<ModelFitState> = {}): ModelFitS
paramsToVary: [],
inputs: null,
result: null,
error: null,
...state
};
};
Expand Down
15 changes: 14 additions & 1 deletion app/static/tests/unit/components/fit/fitTab.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import ActionRequiredMessage from "../../../../src/app/components/ActionRequired
import LoadingSpinner from "../../../../src/app/components/LoadingSpinner.vue";
import FitPlot from "../../../../src/app/components/fit/FitPlot.vue";
import { mockFitState, mockGraphSettingsState } from "../../../mocks";
import { WodinError } from "../../../../src/app/types/responseTypes";
import ErrorInfo from "../../../../src/app/components/ErrorInfo.vue";

describe("Fit Tab", () => {
const getWrapper = (
Expand All @@ -22,7 +24,8 @@ describe("Fit Tab", () => {
fitting = false,
sumOfSquares: number | null = 2.1,
mockFitModel = jest.fn(),
mockSetFitting = jest.fn()
mockSetFitting = jest.fn(),
error: WodinError | null = null
) => {
const store = new Vuex.Store<FitState>({
state: mockFitState(),
Expand All @@ -45,6 +48,7 @@ describe("Fit Tab", () => {
fitting,
sumOfSquares,
fitUpdateRequired,
error,
result: {
solution: jest.fn()
}
Expand Down Expand Up @@ -82,6 +86,7 @@ describe("Fit Tab", () => {
expect(fitPlot.findAll("span").at(0)!.text()).toBe("Iterations: 10");
expect(fitPlot.findAll("span").at(1)!.text()).toBe("Sum of squares: 2.1");
expect(fitPlot.find("#fit-cancelled-msg").exists()).toBe(false);
expect(wrapper.findComponent(ErrorInfo).props("error")).toBe(null);
});

it("renders as expected when fit is running", () => {
Expand Down Expand Up @@ -169,6 +174,14 @@ describe("Fit Tab", () => {
expect(fitPlot.findAll("span").at(1)!.text()).toBe("Sum of squares: 2.1");
});

it("renders model fit error as expected", () => {
const error = { error: "test error", detail: "test detail" };
const wrapper = getWrapper({}, false, {}, 10, true, false, 2.1, jest.fn(), jest.fn(), error);
expect(wrapper.findComponent(ErrorInfo).props("error")).toStrictEqual(error);
expect(wrapper.findComponent(ActionRequiredMessage).props("message"))
.toBe("An error occurred during model fit.");
});

it("dispatches fit action on click button", async () => {
const mockFitModel = jest.fn();
const wrapper = getWrapper({}, false, false, null, null, false, null, mockFitModel);
Expand Down
2 changes: 2 additions & 0 deletions app/static/tests/unit/serialiser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ describe("serialise", () => {

const modelFitState = {
fitting: false,
error: { error: "fit run error", detail: "fit run detail" },
fitUpdateRequired: {
modelChanged: false,
dataChanged: false,
Expand Down Expand Up @@ -482,6 +483,7 @@ describe("serialise", () => {
};

const expectedModelFit = {
error: { error: "fit run error", detail: "fit run detail" },
fitUpdateRequired: {
modelChanged: false,
dataChanged: false,
Expand Down
33 changes: 33 additions & 0 deletions app/static/tests/unit/store/modelFit/actions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,39 @@ describe("ModelFit actions", () => {
expect(mockWodinFit).not.toHaveBeenCalled();
});

it("FitModel commits error thrown during wodinFit", () => {
const runner = {
wodinFit: jest.fn().mockImplementation(() => { throw new Error("TEST ERROR"); }),
wodinFitValue: mockWodinFitValue
} as any;
const errorModelState = mockModelState({
odin: mockOdin,
odinRunnerOde: runner
});

const errorRootState = { ...rootState, model: errorModelState };

const getters = { fitRequirements: {} };
const link = { time: "t", data: "v", model: "S" };
const rootGetters = {
"fitData/link": link,
"fitData/dataEnd": 100
};
const commit = jest.fn();
const dispatch = jest.fn();
(actions[ModelFitAction.FitModel] as any)({
commit, dispatch, state, rootState: errorRootState, rootGetters, getters
});

expect(commit).toHaveBeenCalledTimes(3);
expect(commit.mock.calls[0][0]).toBe(ModelFitMutation.SetFitting);
expect(commit.mock.calls[0][1]).toBe(true);
expect(commit.mock.calls[1][0]).toBe(ModelFitMutation.SetError);
expect(commit.mock.calls[1][1]).toStrictEqual({ error: "Model fit error", detail: "TEST ERROR" });
expect(commit.mock.calls[2][0]).toBe(ModelFitMutation.SetFitting);
expect(commit.mock.calls[2][1]).toBe(false);
});

it("FitModelStep commits expected changes and dispatches further step if not converged", (done) => {
const result = {
converged: false,
Expand Down
16 changes: 16 additions & 0 deletions app/static/tests/unit/store/modelFit/mutations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,22 @@ describe("ModelFit mutations", () => {
expect(state.fitting).toBe(true);
});

it("SetFitting sets error to null if fitting is true", () => {
const error = { error: "test", detail: "test detail" };
const state = mockModelFitState({ error });
mutations.SetFitting(state, false);
expect(state.error).toBe(error);
mutations.SetFitting(state, true);
expect(state.error).toBe(null);
});

it("sets error", () => {
const error = { error: "test", detail: "test detail" };
const state = mockModelFitState();
mutations.SetError(state, error);
expect(state.error).toBe(error);
});

it("sets model fit inputs", () => {
const state = mockModelFitState();
mutations.SetInputs(state, mockInputs);
Expand Down
Loading