Closed jsoules closed 1 year ago
Merging #57 (2c7c571) into main (16c6099) will increase coverage by
1.82%
. The diff coverage is64.12%
.
@@ Coverage Diff @@
## main #57 +/- ##
========================================
+ Coverage 5.64% 7.46% +1.82%
========================================
Files 60 62 +2
Lines 5385 5358 -27
Branches 21 78 +57
========================================
+ Hits 304 400 +96
+ Misses 5081 4958 -123
Flag | Coverage Δ | |
---|---|---|
gui_units | 9.38% <69.70%> (+2.52%) |
:arrow_up: |
svc_units | 2.28% <0.00%> (-0.02%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted Files | Coverage Δ | |
---|---|---|
service/src/logic/OutputManager.ts | 0.00% <0.00%> (ø) |
|
service/src/types/Typeguards.ts | 0.00% <0.00%> (ø) |
|
...c/MCMCMonitorDataManager/MCMCMonitorDataManager.ts | 0.00% <0.00%> (ø) |
|
...rc/MCMCMonitorDataManager/MCMCMonitorTypeguards.ts | 0.00% <0.00%> (ø) |
|
src/MCMCMonitorDataManager/updateChains.ts | 0.00% <0.00%> (ø) |
|
src/MCMCMonitorDataManager/updateSequenceStats.ts | 0.00% <0.00%> (ø) |
|
src/MCMCMonitorDataManager/updateVariableStats.ts | 0.00% <0.00%> (ø) |
|
src/MCMCMonitorDataManager/useMCMCMonitor.ts | 0.00% <0.00%> (ø) |
|
src/MCMCMonitorDataManager/MCMCMonitorData.ts | 100.00% <100.00%> (+37.46%) |
:arrow_up: |
src/MCMCMonitorDataManager/updateSequences.ts | 100.00% <100.00%> (+100.00%) |
:arrow_up: |
... and 1 more |
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
@jsoules This indeed looks simpler. If you've tested it and it works, then I'm in favor of merging and deploying.
Just did one more rebase, given the changes we pushed to main earlier.
I've re-deployed to dev at https://flatironinstitute.github.io/mcmc-monitor/dev/?s=https://lit-bayou-76056.herokuapp.com and will click around on that a bit more just to be absolutely sure.
Seems to work, although you should probably test it while live updating.
Tested while live updating--everything still appears to function as expected. So I'm going to go ahead and merge this & publish.
This PR comprises two major and one minor change.
The major changes are:
MCMCMonitorData
and corresponding reducer, as well as theupdateSequences
file/function.A corresponding minor change was:
MCMCMonitorData
etc) are in their own file separate from the processing logic, and have their own typeguards;index.js
to the type definitions directory in theservice
project to simplify imports;To comment further on the reducer-logic refactor:
updateSequences
(in src/MCMCMonitorDataManager/updateSequences.ts). At sequence update time, this function called reducer actions to update the set of known sequences to include all those implied by the selected variable names/chain IDs/run ID; then make an API request to get updated information for them; then iterate over the results to append new data to the main cache and mark any received result as updated. This logic was difficult to follow and involved repeated updates to the main cache from an asynchronous function that could easily be called multiple times in parallel."setSelectedVariableNames"
,"setSelectedChainIds"
, and"setSelectedRunId"
reducer verbs now have responsibility for ensuring that the cache's list of sequences includes every combination implied by the selections. That is, updatingMCMCMonitor.sequences[]
to include everyrunId/chainId/variableName
combination happens as those selections are made rather than waiting for updates to be requested.updateSequences
function, which can now just query the state cache for sequences in need of update, then make an (async) API request with those values.updateSequences
function. I also expect (but can't demonstrate) that it reduces the risk of race conditions, since asynchronous (and potentially out of order) requests to the reducer are restricted to (presumably serialized) requests to process API responses, as opposed to the potential case where one update function flags a particular sequence as updated during the time that another instance is constructing its API request, etc.Something similar might be doable with running the updated statistics for the chain-sequences and variables, but that's more inherently asynchronous since it's computationally expensive and thus done by a web worker, so I'm not going to touch it right now.
Deployed this branch to dev at https://flatironinstitute.github.io/mcmc-monitor/dev/?s=https://lit-bayou-76056.herokuapp.com