covid-sim icon indicating copy to clipboard operation
covid-sim copied to clipboard

incDeath_X output contains cumDeath_X values?

Open bwynneHEP opened this issue 4 years ago • 3 comments

I've been looking at some of the severity data broken-down into the different age groups. I noticed a minor (but annoying) bug in one of the main outputs I plot: the incDeath_X output, where X takes values from 0 to 16 corresponding to the different age groups.

It seems that these incidence values output the exact duplicate of the cumulative values, and so they rise monotonically throughout a simulation. This does not appear to be true for incDeath_ILI_X, incDeath_SARI_X, or incDeath__Critical_X, and so one can presumably reconstruct the correct value by summing over these three outputs instead. Nonetheless, it would be helpful for the combined value output to be corrected.

I note that the outputs are created here: https://github.com/mrc-ide/covid-sim/blob/master/src/CovidSim.cpp#L4083 using the TSMean[i].incDa[j] value.

Looking at where this value is set: https://github.com/mrc-ide/covid-sim/blob/master/src/CovidSim.cpp#L5033 TimeSeries[n].incDa[i] += (double)StateT[j].cumDa[i]; would appear to be the source of the mistake. However, I note many other values set in a similar way, so perhaps I have misunderstood the calculation?

This seems to only be an issue affecting the formatting of simulation output, and I doubt it would have affected anyone's results without them noticing.

bwynneHEP avatar Jun 11 '20 11:06 bwynneHEP

Hi @bwynneHEP and @weshinsley, yes you're right this does actually output the cumulative incidence. It was a quick hack that we post process in R. However you're right we should change it. Before I do though I need to check that it doesn't ruin anything upstream. Will get to it soon.

dlaydon avatar Jun 15 '20 09:06 dlaydon

Not an urgent problem - as you say, once I knew about it, fixing as a post-process step is easy.

Probably worth a careful look though, as there seemed to be a few incSomething values that were set a similar way (i.e. without the subtraction of the previous step value before the addition of the new one).

I also noticed that the corresponding cumDeath_X output is made by summing over the ILI, SARI and Critical categories at the moment the output is made inside CovidSim.cpp. This might mean that some other ad-hoc fixes have already been applied.

bwynneHEP avatar Jun 15 '20 10:06 bwynneHEP

Yes when we do it properly, we add to the cumulative incidence at every timepoint in the State/StateT/POPVAR structures, then record the incidence in TimeSeries/RESULTS structure in RecordSample / CovidSim.cpp by subtracting the previous timepoint's running total from the current running total. All a little cumbersome but as it works who really cares?! Would be interested to see a cleaner version that preserves the threading if you're interested (no worries if not).

dlaydon avatar Jun 15 '20 17:06 dlaydon