ScottishCovidResponse / SCRCIssueTracking

Central issue tracking repository for all repos in the consortium
6 stars 0 forks source link

Model doesn't run for the correct number of simulation steps #740

Closed peter-t-fox closed 3 years ago

peter-t-fox commented 3 years ago

In each Model implementation, the loop controlling the number of simulation steps is set up as:

    for (int tt{1}; tt < n_sim_steps; ++tt) {

It's not clear why the loop starts at 1: it may be for consistency with the way in which the day_shut variable is interpreted.

In any case, this will result always in one less simulation step than the user expects.

github-actions[bot] commented 3 years ago

Heads up @thibaud-porphyre @peter-t-fox - the "Covid19_EERAModel" label was applied to this issue.

kzscisoft commented 3 years ago

@peter-t-fox I don't think this is a bug, somehow the number of steps is still the number requested even with the loop starting at 1. When I set it to start at 0:

// OriginalModel.cpp
std::cout << "NSIMSTEPS: " << n_sim_steps << std::endl;

// Utilties.cpp : sse_calc
std::cout << simval.size() << "\t" << obsval.size() << std::endl;

Gives:

NSIMSTEPS: 91
92      91
kzscisoft commented 3 years ago

UPDATE: This is directly related to https://github.com/ScottishCovidResponse/SCRCIssueTracking/issues/742. One counterbalances the other, i.e. starting with having a number already in the vector {0} means the loop has to start at 1.

kzscisoft commented 3 years ago

@thibaud-porphyre can you verify if this is a requirement or a bug, the relevant lines of code are:

OriginalModel.cpp: L125 OriginalModel.cpp: L110

Is there a reason for some of the status attributes being empty vectors and others being initialised to {0}?

thibaud-porphyre commented 3 years ago

it is not the bug because we seed the disease first before the loop. This represents the time tt{0}. then we loop from 1 to n_sim_step

peter-t-fox commented 3 years ago

Closing this one, as it is now clearer from the discussion of #742 what is going on.