mrc-4522 Allow user-specified parameter values in Sensitivity #176

EmmaLRussell · 2023-08-24T16:50:16Z

This branch adds a new sensitivity parameter settings variation type, "User", to the existing types ("Percentage" and "Range"). This allows the user to manually specify the parameter values to use in the sensitivity run.

On reflection, "User" maybe isn't a great term to surface to the, err, user. What would be better? "Values"? "Specific"?

This isn't doing any validation that the parameter's central value is between the lowest and highest values specified - should it?

Key changes:

userValues number array added to the settings type to hold these values
EditParamSettings dialog updated to support editing these values using the TagInput, plus read-only display in the options tab. The green alert showing generated values is not displayed for user values (as would be showing the same values twice) - I guess we could just show this in the read-only tab view but it feels better to have the user values displayed like the other user settings, and keep the green box for generated values.
We don't need to generate batchPars using odin's helper methods for this variation type as we already know the parameter values to use, so the validation and run sensitivity code has been modified to allow this.
To be consistent with the other variation types, a minimum of two user values are required.
Values are de-duped and sorted as they are entered.

codecov · 2023-08-24T23:56:34Z

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (bb536f4) 99.97% compared to head (7d45e46) 99.97%.
Report is 15 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #176   +/-   ##
=======================================
  Coverage   99.97%   99.97%           
=======================================
  Files         148      148           
  Lines        3412     3436   +24     
  Branches      482      491    +9     
=======================================
+ Hits         3411     3435   +24     
  Misses          1        1

Files Changed	Coverage Δ
.../src/app/components/options/SensitivityOptions.vue	`100.00% <ø> (ø)`
...pp/static/src/app/store/sensitivity/sensitivity.ts	`100.00% <ø> (ø)`
...c/src/app/components/options/EditParamSettings.vue	`100.00% <100.00%> (ø)`
app/static/src/app/components/options/TagInput.vue	`100.00% <100.00%> (ø)`
app/static/src/app/store/sensitivity/state.ts	`100.00% <100.00%> (ø)`
app/static/src/app/utils.ts	`100.00% <100.00%> (ø)`

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

richfitz · 2023-08-25T09:10:23Z

On reflection, "User" maybe isn't a great term to surface to the, err, user. What would be better? "Values"? "Specific"?

"Custom" perhaps?

EmmaLRussell · 2023-08-25T09:11:51Z

On reflection, "User" maybe isn't a great term to surface to the, err, user. What would be better? "Values"? "Specific"?

"Custom" perhaps?

Yeah, that's probably the word I was looking for! Will change to that!

EmmaLRussell · 2023-08-25T10:05:44Z

OK, that should be working now, ready to review.

richfitz

This works

richfitz · 2023-08-25T10:16:39Z

app/static/src/app/components/options/TagInput.vue

-        tags: Array as PropType<Tag[] | null>
+        tags: Array as PropType<Tag[] | null>,
+        numericOnly: {
+            type: Boolean,


is there a difference between Boolean and bool? Or am I getting confused with different languages?

It's how Vue defines the types of its props, not quite the same as the Typescript types

richfitz · 2023-08-25T10:19:39Z

app/static/src/app/components/options/SensitivityOptions.vue

@@ -5,18 +5,25 @@
        <button class="btn btn-primary mb-4 float-end" @click="toggleEdit(true)">Edit</button>
        <ul>
          <li><strong>Parameter:</strong> {{settings.parameterToVary}}</li>
-          <li><strong>Scale Type:</strong> {{ settings.scaleType }}</li>
+          <li v-if="settings.variationType !== 'Custom'"><strong>Scale Type:</strong> {{ settings.scaleType }}</li>


I feel this should now move below the variation type because then the dependency among the options is easier to work with

Ok, so scale type below variation type, as it isn't relevant for custom variation type.

richfitz · 2023-08-25T10:20:13Z

app/static/src/app/components/options/SensitivityOptions.vue

            <li><strong>From:</strong> {{ settings.rangeFrom }}</li>
            <li><strong>To:</strong> {{ settings.rangeTo }}</li>
          </template>
-          <li><strong>Number of runs:</strong> {{ settings.numberOfRuns}}</li>
+          <li v-if="settings.variationType === 'Custom'">
+            <strong>Values:</strong>


I was looking for some hint as to what my central value was. Do we want to encourage the users to bracket it?

Yeah, I wondered about that. Do we want to enforce or just encourage?

encourage seems enough - just knowing where it is would be nice

richfitz · 2023-08-25T10:21:51Z

app/static/src/app/components/options/EditParamSettings.vue

+            return batchParsResult.value?.error;
+        });
+        const updateUserValues = (newValues: number[]) => {
+            // sort and remove duplicates


when I entered a duplciate it ended up with a red underline and was not included but no error displayed

I also wonder about deferring sort until save? it's a bit alarming atm

The red underline seems to be a bit of a feature of the third party tags input, I think it can happen in the advanced settings too, I'll see if I can get rid of it, and defer sorting

M-Kusumgar

I think the code looks good, youre in the middle of changes, should be good once all those are implemented, you can try handling duplicate values with the on-error event from the tags component, however the red underline will still show up, the class you probably want to modify is
v3ti-new-tag v3ti-new-tag--error, you can change behaviour with this!

You can also wipe their duplicate value and stuff using a v-model on the tags input and setting that to empty string on the on-error event, once the red underline styling is gone, we can control a tooltip!

EmmaLRussell · 2023-08-25T15:00:58Z

scale type now displayed below variation type on editor and tab
central value displayed as it is for Range type
sort happens on OK, not while typing
as discussed with @M-Kusumgar the third party tags input isn't great at coping with duplicates at all, so if the user does enter a duplicate we now just remove it immediately (no underlined red text).

EmmaLRussell added 9 commits August 24, 2023 17:00

configure user value variation type

bda7c21

add numeric only mode to TagInput

efab479

show error if less than 2 user values

c9a3642

do not show scale type in user mode

004be28

add editor render unit test

4e215cf

unit tests

e4b424f

test fixes

5c42f84

lint

f12f236

getter test fix

0783bee

EmmaLRussell added 2 commits August 25, 2023 09:50

e2e test

13ad200

lint

271eed2

EmmaLRussell marked this pull request as ready for review August 25, 2023 09:06

EmmaLRussell requested review from M-Kusumgar and richfitz August 25, 2023 09:06

EmmaLRussell added 2 commits August 25, 2023 10:32

rename User variation type to Custom

864cf95

test fix

b783bb8

richfitz requested changes Aug 25, 2023

View reviewed changes

EmmaLRussell added 3 commits August 25, 2023 11:55

Move scale type below variation type on editor and tab

4f6621f

show central value

20f33bb

lint

b419569

M-Kusumgar reviewed Aug 25, 2023

View reviewed changes

sort on OK, and remove duplicate value from TagInput

7d45e46

EmmaLRussell requested review from richfitz and M-Kusumgar August 25, 2023 15:08

EmmaLRussell mentioned this pull request Aug 30, 2023

mrc-4456_single_edit Configuration and settings for multi-sensitivity #177

Merged

richfitz approved these changes Aug 30, 2023

View reviewed changes

M-Kusumgar approved these changes Aug 30, 2023

View reviewed changes

EmmaLRussell merged commit fdd2ab9 into main Aug 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mrc-4522 Allow user-specified parameter values in Sensitivity #176

mrc-4522 Allow user-specified parameter values in Sensitivity #176

EmmaLRussell commented Aug 24, 2023 •

edited

Loading

codecov bot commented Aug 24, 2023 •

edited

Loading

richfitz commented Aug 25, 2023

EmmaLRussell commented Aug 25, 2023

EmmaLRussell commented Aug 25, 2023

richfitz left a comment

richfitz Aug 25, 2023

EmmaLRussell Aug 25, 2023

richfitz Aug 25, 2023

EmmaLRussell Aug 25, 2023

richfitz Aug 25, 2023

EmmaLRussell Aug 25, 2023

richfitz Aug 25, 2023

richfitz Aug 25, 2023

richfitz Aug 25, 2023

EmmaLRussell Aug 25, 2023

M-Kusumgar left a comment •

edited

Loading

EmmaLRussell commented Aug 25, 2023

mrc-4522 Allow user-specified parameter values in Sensitivity #176

mrc-4522 Allow user-specified parameter values in Sensitivity #176

Conversation

EmmaLRussell commented Aug 24, 2023 • edited Loading

codecov bot commented Aug 24, 2023 • edited Loading

Codecov Report

richfitz commented Aug 25, 2023

EmmaLRussell commented Aug 25, 2023

EmmaLRussell commented Aug 25, 2023

richfitz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

M-Kusumgar left a comment • edited Loading

Choose a reason for hiding this comment

EmmaLRussell commented Aug 25, 2023

EmmaLRussell commented Aug 24, 2023 •

edited

Loading

codecov bot commented Aug 24, 2023 •

edited

Loading

M-Kusumgar left a comment •

edited

Loading