-
Notifications
You must be signed in to change notification settings - Fork 254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
P.NC initialisation #337
Comments
Do you mean the line in SetupModel.cpp? |
Yes, that's it. |
Fixed incorrect file name, thanks for highlighting. |
I've been putting together some minimal param files and hit this. (Most values mirror the examples, but I've decreased Infectious Period to avoid issues validating MAX_INFECTIOUS_STEPS and I reduced sampling time and population in the hope of getting something that runs quickly.) While running, I see the following logged:
Here are the param files I've been putting together in case you want to replicate: pre-param file
param file
Generally I think it would be beneficial to support some smaller/simpler test cases anyway, and this is one rough edge in that story. |
Another observation I would like to raise here is that this particular computation should really be avoided in the first place (from #376). The user should really only have to provide From that, the struct Param {
// ... irrelevant fields omitted
// they should be default-initialized to `0` to be some sentinel value that
// does not make sense - you can't run the simulation with 0 grids?
uint64_t cell_grid_width;
uint64_t cell_grid_height;
uint64_t cells_count() const {
assert (cell_grid_width > 0 && cell_grid_height > 0);
// should deal with uint64_t * uint64_t overflows
cell_grid_width * cell_grid_height
}
} In |
If you do not provide a density file, you hit an else block in SetupModel.cpp which starts by setting P.ncw = P.nch = (int)sqrt((double)P.NC);
Unless I've missed something, at this point P.NC will be -1, as set unconditionally in ReadParams.
This code path is presumably unused, but it might be worth completely removing support for no density file. If keeping support for no-density-file simulation, perhaps it could use the cell width and the bounding box (generated from the population size) to calculate appropriate P.ncw and P.nch. In that case, it would be good to add a regression test which doesn't use a density file too.
The text was updated successfully, but these errors were encountered: