Export pivot growth advantage #127
Conversation
This config value isn't used in the automated workflows because it overrides with the `--pivot` flag. However, nice to keep things in sync for testing the script. Follow up to #124
Previously this was assumed to be the last value in the `variants` list.¹ Adding an explicit metadata field for `pivot` so that we can remove this assumption in the viz app. ¹ <nextstrain/forecasts-viz@661df8b>
Export hard-coded value of 1.0 for the growth advantage of the pivot variant according to <#126> There may be a better way to accomplish this with evofr methods, but I'm unfamiliar with the models so will leave that as a potential later improvement.
I realized in looking into including the pivot GA plots in forecasts-viz that the model data includes the `pivot` value. Use the `pivot` value from the model data instead of hard-coding it so it won't get out-of-sync when we update the pivots.
|
Trial run successfully completed and results can be viewed at
I was partially correct. Pivot (24F) is not showing up on the x-axis and the legend of the GA plots, but the 1.0 value is plotted on the left side of the y-axis 🤦♀️
I can fix the plot in forecasts-viz and update the viz library in this PR as well. |
| @@ -20,6 +20,10 @@ function App() { | |||
| const mlrCladesData = useModelData(mlrCladesConfig); | |||
| const mlrLineagesData = useModelData(mlrLineagesConfig); | |||
|
|
|||
| const mlrCladesPivotRaw = mlrCladesData?.modelData?.get('pivot') || "loading"; | |||
There was a problem hiding this comment.
This is very clever! Adding metadata.pivot and using this to dynamically write out legend info should be really nice for maintenance.
The pivot is exported in the model JSON in nextstrain/forecasts-ncov#127 Use the explicit pivot if it's available, otherwise continue to use the final entry in the model's list of variants as the pivot.
The model JSONs will include GA values for the pivot variant as of <nextstrain/forecasts-ncov#127>. Update the Graph's xDomain and the Legend to include the pivot variants to properly plot the pivot variant values.
|
This looks great to me @joverlee521! I like the additional touch of I agree that this shouldn't be merged however until the corresponding change is made to |
|
Updated the viz app in b2c353f and locally looks good to me
Merging so these changes will get incorporated in tonight's model run. |
Blocked on merge of nextstrain/forecasts-viz#24Description of proposed changes
Export the growth advantage for the pivot variant with a hard-coded value of 1.0.
There may be a better way to accomplish this with evofr methods, but I'm unfamiliar with the models so will leave that as a potential later improvement.
Note the pivot does not show up in the viz app because the pivot was purposely excluded in nextstrain/forecasts-viz@661df8b. Will need to update forecasts-viz and then update the viz library in this repo to plot the pivot GA.
Related issue(s)
Resolves #126
Checklist