Skip to content

Commit

Permalink
Merge pull request #188 from mrc-ide/mrc-4562
Browse files Browse the repository at this point in the history
mrc-4562 Filter multi-sensitivity parameter settings options to those which are available
  • Loading branch information
EmmaLRussell authored Oct 12, 2023
2 parents 742ea7f + e7caddc commit 32aa9d5
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 21 deletions.
1 change: 1 addition & 0 deletions app/static/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ module.exports = {
"arrow-body-style": "off",
"import/prefer-default-export": "off",
"@typescript-eslint/no-non-null-assertion": "off",
"no-plusplus": "off",
"no-shadow": "off",
"@typescript-eslint/no-shadow": ["error"],
"no-underscore-dangle": "off",
Expand Down
14 changes: 8 additions & 6 deletions app/static/src/app/components/options/EditParamSettings.vue
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,12 @@
<label class="col-form-label">Parameter to vary</label>
</div>
<div class="col-6">
<select class="form-select" v-model="settingsInternal.parameterToVary">
<select v-if="paramNames.length > 1"
class="form-select edit-param-select"
v-model="settingsInternal.parameterToVary">
<option v-for="param in paramNames" :key="param">{{param}}</option>
</select>
<div v-else class="col-form-label param-name-label">{{ settingsInternal.parameterToVary }}</div>
</div>
</div>
<div class="row mt-2" id="edit-variation-type">
Expand Down Expand Up @@ -144,6 +147,10 @@ export default defineComponent({
type: Boolean,
required: true
},
paramNames: {
type: Array as PropType<string[]>,
required: true
},
paramSettings: {
type: Object as PropType<SensitivityParameterSettings>,
required: false
Expand All @@ -159,10 +166,6 @@ export default defineComponent({
const store = useStore();
const settingsInternal = reactive({} as SensitivityParameterSettings);
const paramNames = computed(() => {
return store.state.run.parameterValues ? Object.keys(store.state.run.parameterValues) : [];
});
const scaleValues = Object.keys(SensitivityScaleType);
const variationTypeValues = Object.keys(SensitivityVariationType);
Expand Down Expand Up @@ -208,7 +211,6 @@ export default defineComponent({
};
return {
paramNames,
scaleValues,
variationTypeValues,
settingsInternal,
Expand Down
15 changes: 15 additions & 0 deletions app/static/src/app/components/options/SensitivityOptions.vue
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
</vertical-collapse>
<edit-param-settings :open="editOpen"
:param-settings="settingsToEdit"
:param-names="paramNamesForSettingsEdit"
@close="closeEdit"
@update="editSettings"></edit-param-settings>
</template>
Expand Down Expand Up @@ -124,6 +125,19 @@ export default defineComponent({
return editSettingsIdx.value === null ? null : allSettings.value[editSettingsIdx.value];
});
const paramNamesForSettingsEdit = computed(() => {
const allParamNames = store.state.run.parameterValues ? Object.keys(store.state.run.parameterValues) : [];
if (props.multiSensitivity) {
// return all params which are either unused or are the param for the settings to edit
const editIdx = editSettingsIdx.value;
const editParam = editIdx !== null && (allSettings.value[editIdx]?.parameterToVary || null);
// check if a given param is that for the settings being edited
const isEditParam = (p: string) => !addingParamSettings.value && editParam !== null && p === editParam;
return allParamNames.filter((p) => paramsWithoutSettings.value.includes(p) || isEditParam(p));
}
return allParamNames;
});
const openEdit = (settingsIdx: number) => {
editSettingsIdx.value = settingsIdx;
editOpen.value = true;
Expand Down Expand Up @@ -174,6 +188,7 @@ export default defineComponent({
allSettings,
settingsToEdit,
paramsWithoutSettings,
paramNamesForSettingsEdit,
canDeleteSettings,
showOptions,
compileModelMessage,
Expand Down
29 changes: 20 additions & 9 deletions app/static/tests/e2e/multiSensitivity.etest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,21 @@ test.describe("Multi-sensitivity tests", () => {
await expect(await section.locator(".alert-success").innerText()).toBe(` ${values}`);
};

const expectModalParamSettings = async (page: Page, param: string, scale: SensitivityScaleType,
variation: SensitivityVariationType, percentage: number | null, from: number | null, central: number | null,
to: number | null, runs: number, values: string) => {
const expectModalParamSettings = async (page: Page, param: string, paramNames: string[],
scale: SensitivityScaleType, variation: SensitivityVariationType, percentage: number | null,
from: number | null, central: number | null, to: number | null, runs: number, values: string) => {
await expect(await page.inputValue("#edit-param-to-vary select")).toBe(param);

if (paramNames.length === 1) {
await expect(await page.innerText("#edit-param-to-vary div.param-name-label")).toBe(param);
} else {
await expect(await page.inputValue("#edit-param-to-vary select")).toBe(param);
const options = await page.locator("#edit-param-to-vary select option");
for (let i = 0; i < paramNames.length; i++) {
await expect(await options.nth(i).innerText()).toBe(paramNames[i]);
}
}

await expect(await page.inputValue("#edit-variation-type select")).toBe(variation);
await expect(await page.inputValue("#edit-scale-type select")).toBe(scale);
if (variation === SensitivityVariationType.Percentage) {
Expand All @@ -57,14 +68,14 @@ test.describe("Multi-sensitivity tests", () => {
await expect(await page.locator(".sensitivity-options-settings").count()).toBe(1);
await expectOptionsTabParamSettings(page, 1, "beta", SensitivityScaleType.Arithmetic,
SensitivityVariationType.Percentage, 10, null, null, 10, "3.600, 3.689, 3.778, ..., 4.400");

// should be no delete button on first param settings
await expect(await page.locator(".delete-param-to-vary").count()).toBe(0);

// open edit param settings dialog and check values
await page.click(":nth-match(.edit-param-settings, 1)");
const allParams = ["beta", "I0", "N", "sigma"];
await expect(await page.locator("#edit-param").count()).toBe(1);
await expectModalParamSettings(page, "beta", SensitivityScaleType.Arithmetic,
await expectModalParamSettings(page, "beta", allParams, SensitivityScaleType.Arithmetic,
SensitivityVariationType.Percentage, 10, null, null, null, 10, "3.600, 3.689, 3.778, ..., 4.400");

// edit default params settings
Expand All @@ -80,7 +91,7 @@ test.describe("Multi-sensitivity tests", () => {

// Add new settings - save defaults
await page.click("#add-param-settings");
await expectModalParamSettings(page, "I0", SensitivityScaleType.Arithmetic,
await expectModalParamSettings(page, "I0", ["I0", "N", "sigma"], SensitivityScaleType.Arithmetic,
SensitivityVariationType.Percentage, 10, null, null, null, 10, "0.900, 0.922, 0.944, ..., 1.100");
await page.click("#ok-settings");
await expect(await page.locator(".sensitivity-options-settings").count()).toBe(2);
Expand All @@ -89,17 +100,17 @@ test.describe("Multi-sensitivity tests", () => {

// Add another new settings - make changes before saving
await page.click("#add-param-settings");
await expectModalParamSettings(page, "N", SensitivityScaleType.Arithmetic,
await expectModalParamSettings(page, "N", ["N", "sigma"], SensitivityScaleType.Arithmetic,
SensitivityVariationType.Percentage, 10, null, null, null, 10,
"900000.000, 922222.222, 944444.444, ..., 1100000.000");

// change a parameter
await page.selectOption("#edit-param-to-vary select", "sigma");
await expectModalParamSettings(page, "sigma", SensitivityScaleType.Arithmetic,
await expectModalParamSettings(page, "sigma", ["N", "sigma"], SensitivityScaleType.Arithmetic,
SensitivityVariationType.Percentage, 10, null, null, null, 10, "1.800, 1.844, 1.889, ..., 2.200");
// change percent
await page.fill("#edit-percent input", "5");
await expectModalParamSettings(page, "sigma", SensitivityScaleType.Arithmetic,
await expectModalParamSettings(page, "sigma", ["N", "sigma"], SensitivityScaleType.Arithmetic,
SensitivityVariationType.Percentage, 5, null, null, null, 10, "1.900, 1.922, 1.944, ..., 2.100");

// Save
Expand Down
15 changes: 10 additions & 5 deletions app/static/tests/unit/components/options/editParamSettings.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import Vuex from "vuex";
import { mount, VueWrapper } from "@vue/test-utils";
import { nextTick } from "vue";
import {
SensitivityParameterSettings,
SensitivityScaleType,
Expand All @@ -12,7 +11,6 @@ import {
import EditParamSettings from "../../../../src/app/components/options/EditParamSettings.vue";
import { BasicState } from "../../../../src/app/store/basic/state";
import NumericInput from "../../../../src/app/components/options/NumericInput.vue";
import { SensitivityMutation } from "../../../../src/app/store/sensitivity/mutations";
import SensitivityParamValues from "../../../../src/app/components/options/SensitivityParamValues.vue";
import { expectCloseNumericArray } from "../../../testUtils";
import TagInput from "../../../../src/app/components/options/TagInput.vue";
Expand Down Expand Up @@ -61,7 +59,8 @@ describe("EditParamSettings", () => {

const parameterValues = { A: 1, B: 2, C: 3 };

const getWrapper = async (paramSettings: SensitivityParameterSettings, open = true) => {
const getWrapper = async (paramSettings: SensitivityParameterSettings, open = true,
paramNames = ["A", "B", "C"]) => {
const store = new Vuex.Store<BasicState>({
state: mockBasicState(),
modules: {
Expand All @@ -86,6 +85,7 @@ describe("EditParamSettings", () => {
},
props: {
open: false,
paramNames,
paramSettings
}
});
Expand Down Expand Up @@ -118,6 +118,7 @@ describe("EditParamSettings", () => {
expect(paramOptions.at(0)!.text()).toBe("A");
expect(paramOptions.at(1)!.text()).toBe("B");
expect(paramOptions.at(2)!.text()).toBe("C");
expect(wrapper.find("#edit-param-to-vary .param-name-label").exists()).toBe(false);

expect(wrapper.find("#edit-scale-type label").text()).toBe("Scale type");
const scaleSelect = wrapper.find("#edit-scale-type select");
Expand Down Expand Up @@ -294,18 +295,22 @@ describe("EditParamSettings", () => {
});

it("updates values from TagInput", async () => {
const mockSetParamSettings = jest.fn();
const wrapper = await getWrapper(customSettings, true);
const expectedValues = [-1, 0, 1];
wrapper.findComponent(TagInput).vm.$emit("update", expectedValues);
expect((wrapper.vm as any).settingsInternal.customValues).toStrictEqual([-1, 0, 1]);
});

it("sorts custom values on update", async () => {
const mockSetParamSettings = jest.fn();
const wrapper = await getWrapper(customSettings, true);
wrapper.findComponent(TagInput).vm.$emit("update", [5, 1, 0, -2, -1]);
await wrapper.find("#ok-settings").trigger("click");
expect((wrapper.emitted("update")![0] as any)[0].customValues).toStrictEqual([-2, -1, 0, 1, 5]);
});

it("renders parameter to vary as text rather than select if only one option", async () => {
const wrapper = await getWrapper(percentSettings, true, ["B"]);
expect(wrapper.find("#edit-param-to-vary select").exists()).toBe(false);
expect(wrapper.find("#edit-param-to-vary .param-name-label").text()).toBe("B");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ describe("SensitivityOptions", () => {
run: {
namespaced: true,
state: {
parameterValues: {}
parameterValues: { A: 1, B: 2 }
}
}
} as any
Expand Down Expand Up @@ -280,6 +280,7 @@ describe("SensitivityOptions", () => {
const editParamSettings = wrapper.findComponent(EditParamSettings);
expect(editParamSettings.props("open")).toBe(true);
expect(editParamSettings.props("paramSettings")).toStrictEqual(percentSettings);
expect(editParamSettings.props("paramNames")).toStrictEqual(["A", "B"]);

await editParamSettings.vm.$emit("close");
expect(editParamSettings.props("open")).toBe(false);
Expand All @@ -292,6 +293,7 @@ describe("SensitivityOptions", () => {
const editParamSettings = wrapper.findComponent(EditParamSettings);
expect(editParamSettings.props("open")).toBe(true);
expect(editParamSettings.props("paramSettings")).toStrictEqual(percentSettings);
expect(editParamSettings.props("paramNames")).toStrictEqual(["A", "C", "D"]);

await editParamSettings.vm.$emit("close");
expect(editParamSettings.props("open")).toBe(false);
Expand Down Expand Up @@ -402,4 +404,24 @@ describe("SensitivityOptions", () => {
]);
expect(wrapper.find("#add-param-settings").exists()).toBe(false);
});

it("removes parameter names from editor which already have multi-sensitivity, when Adding", async () => {
const wrapper = getWrapperForMultiSensitivity([
percentSettings, // param A
rangeSettings, // param B
customSettings // param C
]);
await wrapper.find("#add-param-settings").trigger("click");
expect(wrapper.findComponent(EditParamSettings).props("paramNames")).toStrictEqual(["D"]);
});

it("removes non-current parameter names from editor which already have settings, when Editing", async () => {
const wrapper = getWrapperForMultiSensitivity([
percentSettings, // param A
rangeSettings, // param B
customSettings // param C
]);
await wrapper.findAll(".edit-param-settings")[0].trigger("click");
expect(wrapper.findComponent(EditParamSettings).props("paramNames")).toStrictEqual(["A", "D"]);
});
});

0 comments on commit 32aa9d5

Please sign in to comment.