SamuelBrand1 / KenyaCoV

This repo contains the source code KenyaCoV, a simulation package for forecasting SARS-CoV-2 transmission in Kenya.
MIT License
6 stars 0 forks source link

SparseMatrix BoundsError in change_matrix function in src/dynamics.jl #14

Open TheHelenofTroy opened 4 years ago

TheHelenofTroy commented 4 years ago

On this line of src/dynamics.jl, Julia reports a BoundsError on SparseMatrix. Seems like the function is trying to access elements out of the defined matrix index range and the loop is not checking for such index bound limits.

See below for error message and stacktrace, when this error occurred while running test/run_KenyaCoV_agestructuredmodel.jl.

ERROR: LoadError: BoundsError: attempt to access 4080×2720 SparseArrays.SparseMatrixCSC{Int64,Int64} at index [6393, 1599] Stacktrace: [1] setindex!(::SparseArrays.SparseMatrixCSC{Int64,Int64}, ::Int64, ::Int64, ::Int64) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.0/SparseArrays/src/sparsematrix.jl:2347 [2] change_matrix(::SparseArrays.SparseMatrixCSC{Int64,Int64}) at /home/KenyaCoV/src/dynamics.jl:167 [3] model_ingredients_from_data(::String, ::String, ::String) at /home/KenyaCoV/src/transmissionmodel.jl:118 [4] top-level scope at none:0

SamuelBrand1 commented 4 years ago

Hi TheHelenOfTroy,

We are currently completely changing the KenyaCoV model to include the new scientific knowledge about SARS-CoV-2 that had amassed since the first preprint model.

The errors you’re detecting are due to breaking changes in the code base as we transition to a more realistic model that includes more spatial regions, more epidemiology, more resolved data etc. This change is lead from the team in Kenya.

We will put note on the preprint formally withdrawing the old model and watch this space.

All the best,

Sam Brand

Sent from my iPhone

On 16 May 2020, at 06:14, TheHelenofTroy notifications@github.com wrote:

On this line of src/dynamics.jl, Julia reports a BoundsError on SparseMatrix. Seems like the function is trying to access elements out of the defined matrix index range and the loop is not checking for such index bound limits.

See below for error message and stacktrace, when this error occurred while running test/run_KenyaCoV_agestructuredmodel.jl.

ERROR: LoadError: BoundsError: attempt to access 4080×2720 SparseArrays.SparseMatrixCSC{Int64,Int64} at index [6393, 1599] Stacktrace: [1] setindex!(::SparseArrays.SparseMatrixCSC{Int64,Int64}, ::Int64, ::Int64, ::Int64) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.0/SparseArrays/src/sparsematrix.jl:2347 [2] change_matrix(::SparseArrays.SparseMatrixCSC{Int64,Int64}) at /home/KenyaCoV/src/dynamics.jl:167 [3] model_ingredients_from_data(::String, ::String, ::String) at /home/KenyaCoV/src/transmissionmodel.jl:118 [4] top-level scope at none:0

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

TheHelenofTroy commented 4 years ago

Great to know. Thanks for the headsup! Really appreciate the hardwork you put in. Looking forward to the new model coming out! Is this code base going to be updated frequently as you are building the new model, or shall we expect major pushes every once in a while? Again, thanks for the awesome work!

SamuelBrand1 commented 4 years ago

Hi there,

The updates will be frequent but not so breaking in the future. Also, the code structure will be more modular and flexible.

For example, preprint version is based on 20 “risk regions” from an old paper analysing population grouping in Kenya, events in these regions then get attributed to counties. In the new version, counties get their own interactive dynamics.

However, in code there are too many bits where the number of spatial regions are a global of the KenyaCoV module scope. This is fine(ish) with only power-users like our team modifying the code on the fly... terrible for other people to use. Many other instances of issues like this in the code base.

Thanks for your support. On Monday I’ll submit a note to the preprint, since atm it doesn’t do what it says it does (and used to do). We probably should have kept original version as a functioning legacy model, and this is something we will do in the future.

Sent from my iPhone

On 16 May 2020, at 08:13, TheHelenofTroy notifications@github.com wrote:

Great to know. Thanks for the headsup! Really appreciate the hardwork you put in. Looking forward to the new model coming out! Is this code base going to be updated frequently as you are building the new model, or shall we expect major pushes every once in a while? Again, thanks for the awesome work!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

TheHelenofTroy commented 4 years ago

Hi Sam,

Thanks for sharing the plan going forward and giving us some glimpse into the details. The improvement of modularity and reproducibility will be greatly beneficial to the community for sure. Really appreciate the work your team has done, and looking forward to seeing the future evolution of it.

Best regards, THoT