ncsu-landscape-dynamics / pops-core Goto Github PK

Although we have some rough instructions in STYLE_GUIDE there is no complete definition and no checks to ensure them. Add rules and tools at least to keep whitespace consistent.

The deploy step of the gh-pages job fails with

  /usr/bin/ssh-add /home/runner/.ssh/github
  Error loading key "/home/runner/.ssh/github": invalid format
  ##[error]Action failed with "The process '/usr/bin/ssh-add' failed with exit code 1"

which does not seem to be on our end, but it appeared after migration of gh-pages from Travis to GH Actions and rebuild does not help.

Fortunately, this does not influence the older version of the documentation, so that's still accessible and can be linked elsewhere if needed.

The current implementation of movements is not ideal. We would like to create a class that is more similar to treatments to allow for more flexibility.

should weather_step just use current_index

Based on comment by @ChrisJones687 in #19:

Kernels to implement

(comes from data on seed dispersal but very applicable)
...

Available in C++11

• Exponential: single parameter should be 1/(mean distance (this is calculated in MCMC))
• Gamma: double parameter (shape (1.0) and scale(estimated))
• Cauchy: double parameter (shift (0.0) and scale(estimated))
• Weibull: double parameter (shape (1.0) and scale (estimated))
• Normal: double Parameter (mean (0.0) and sd (estimated))
• Lognormal: double parameter (mean (0.0) and sd (estimated))

Available in Boost, but not available in standard C++

Available in boost C++ math library

• WALD or Inverse Gaussian
• Logistic distribution

Others to be implemented, not in C++11 or Boost

• 2Dt
• Power Law (Cauchy is one variant of a power law)
• log hyperbolic secant
• Exponential Power

#19 is now resolved by #27, so this can be implemented.

Add in the resistant pool(s). The idea here is that it is another compartment in the model but one that is ignored for most calculations of spread but is used more for treatment using pesticides (related to #33 ) or if resistant is obtained when an individual recovers from a disease (unaware of this in plant systems but happens often in human and animal diseases).

This will include a function to return hosts from the resistant pool to the susceptible pool after a defined length of time. The question is whether option 1) one large pool that could be imported and exported with a bunch of small trackers with separate return times option or 2) have a bunch of separate pools that are linked to their return times that can be imported/exported makes the most sense and is the most flexible.

Started something here in a rpops branch. We just need to think through the best way to implement this in an easy to use and flexible manner.

There seems to be problem in current treatment scheduling, specifically it shows up when run weekly and treatment is scheduled at the end of the year, e.g. scheduled in the first year 12-31. The condition as it is now:
https://github.com/ncsu-landscape-dynamics/PoPS/blob/master/treatments.hpp#L118

results in completely skipping the 12-31 treatment. It's caused somehow by the syncing to 1/1 when run weekly. I am not sure how to properly fix it, maybe line
https://github.com/ncsu-landscape-dynamics/PoPS/blob/master/treatments.hpp#L117
needs to add actual 7 days and not use the special increased_by_week function, but I would have to make more tests if that actual works for all cases.

The new scheduling mechanism shouldn't have this problem, so if we adopt it, I don't think it's worth spending time fixing this.

This update will move susceptible individuals to exposed for a latent period (x number of days) and after that latent period the exposed group will become infected.

To compute spread rate, I suggest to implement function which computes the most northern, southern, eastern and western infection to get an infection bbox. This would be applied to the initial infection raster and the last infection rasters. The average of values of all the stochastic runs would be compared to the values from initial infection to compute spread rate.
GRASS r.pops.spread would probably export a vector with 2 bboxes (initial and averaged max spread from runs), not sure what's the best for rpops.

What needs to be done

A simple citation can be in the readme file. The question is if it is a paper or the software itself or both.

Current state

The readmes mention Meentemeyer (2011) and Tonini (2017) papers.

r.pops.spread is has the references and authors sections in the documentation as all GRASS GIS modules do:

https://grass.osgeo.org/grass78/manuals/addons/r.pops.spread.html#references

and additionally, the repo experimentally includes also Citation File Format (CFF) file:

https://github.com/ncsu-landscape-dynamics/r.pops.spread/blob/master/CITATION.cff

I used CFF after an investigation into software citations. It is a good bet as it is relatively simple to read and write for humans but it can handle more complex cases. The issue is that for both R and GRASS GIS, it duplicates what you need/want to have elsewhere.

rpops has authors in the R's DESCRIPTION file:

https://github.com/ncsu-landscape-dynamics/rpops/blob/master/DESCRIPTION

What about interfaces?

I think the interfaces should link to the core repo. How? They already do in the readme, but in terms of citation they can include PoPS Core in the references if we decide to promote citation for the software/repo itself. If not, then same paper used everywhere, nice and simple.

The recommend citation can be just the same one, i.e., PoPS Core, not the interface. This is simple and almost the same as if we use paper everywhere.

CFF has references section which can include both a paper and dependencies. The "message" can explain the relation and the need/recommendation to reference both or all (interface, core, paper).

How does this relate to DOI?

If we get DOI for PoPS Core, then we need to include it in the interfaces. We can also get one DOI for all... A DOI for each repo is probably best supported: GitHub: Making Your Code Citable. It is not 100% clear to me, but perhaps the Concept and Version DOI can be resolved in this way too. (It seems that registering the repo before the release is needed.)

I think the downside of DOIs here is that adding DOI to repo changes the content, so basically invalidating the DOI as a reference to an exact version.

Add the capability for spread rates to be calculated at every output interval rather than just yearly. Should be able to be handled in config similarly to quarantine stats. We should default to handling all of our statistical outputs this way and add a use_spreadrates (or whatever stat) bool to turn off the functionality if we don't need to use it for that case study.

https://github.com/ncsu-landscape-dynamics/PoPS/blob/master/simulation.hpp#L128

or am I missing something?

Add a module to track the detectability of the pest or pathogen based.

To reduce overall stochasticity of the model, optionally remove stochasticity from the disperse step in the Simulation::disperse() function.

Add functionality to compute the following metrics:

• Probability of escaping quarantine
• Year of escape
• Distance to quarantine

Possible implementation (in statistics.hpp):

• New class for quarantine
  • raster input representing quarantine area(s) (0 - no quarantine, 1, 2, 3 ... separate quarantine areas)
  • finds N, S, E, W boundary of the area(s)
  • method that gets infected cell raster and for each infected cells finds out out if inside area, and if yes, computes distance in 4 directions to the boundary. Reports if inside or not and minimum distances in all directions (or just the direction with the minimum distance).
  • aggregate across all quarantine areas (alternatively report separately)
• Function to aggregate the results of this method for multiple runs to compute the probability of escape, report the distribution of min distances.

A good example is total_plants to total_hosts

We already have a CONTRIBUTING file, but we need to add content for first time visitors (not only code, but also issues etc.).

Add final_step as another frequency string for output frequency from string function. This will require an additional scheduler function.

This should work out of the box for cases where only final output is required.

Related: ncsu-landscape-dynamics/rpops#25.

Based on the spotted lanternfly case study it is clear the need for some type of spread via transportation networks. For example, spotted lanternfly spreads long distances along railways by hitchhiking on trains and hopping off along the route. This could be done via a cost matrix (railway corridors speed up spread, forest areas slow spread, etc.

Add in another dispersal module that determines distance and direction via a deterministic process in order. This is useful for testing and uncertainty assessments

The exposed and resistant classes are currently not being used in the movement function. This should be fixed with conversion of movements to its own class in #87.

Currently, the exposed class is not being removed during treatment. This should be an easy fix using treatment.manage_mortality and I have a version of this running locally in rpops and will push here once I confirm it works. We may want to rename treatments.manage_mortality but don't have a good solution.

There is a long list of warnings reported during the compilation. Reduce the list to make the warnings more useful and harder to ignore or miss. Possibly reduce the number to zero even if ignoring a particular warning is required.

Need to add #include to both of these header files for rpops.

Add season class for seasonality and make season class work even if selected season start month is later than end month (e.g. November to May)

Now misses current things, contains very old info.

DAYMET drops December 31st from leap years therefore when simulating leap years using DAYMET as our data source we should use that convention as well otherwise our weather data will not match up with our simulation time. Only really an issue when running over long time periods.

Add capability to pass management polygons to modify host map for susceptible and infected.

I ran into this really weird problem that nothing spreads and it is related to the size (or rather aspect ratio) of the region. The spores seems to be generated in function generate but then in variable dispersers there are only zeros. In test_simulation.cpp you can use these lines

Raster<int> infected = {{0, 0, 0, 5, 5}, {0, 0, 0, 5, 5}, {0, 0, 0, 0, 0}};
Raster<int> mortality_tracker = {{0, 0, 0, 0, 0}, {0, 0, 0, 0, 0}, {0, 0, 0, 0, 0}};
Raster<int> susceptible = {{10, 6, 8, 8, 8}, {14, 15, 12, 12, 12}, {14, 15, 12, 12, 12}};
Raster<int> total_plants = {{15, 6, 10, 10, 10}, {14, 15, 14, 14, 10}, {14, 15, 14, 14, 10}};
Raster<double> temperature = {{5, 0, 5, 0, 0}, {0, 0, 0, 0, 0}, {0, 0, 0, 0, 0}};
Raster<double> weather_coefficient = {{0.6, 0.8, 0.7, 0.7, 0.7}, {0.2, 0.8, 0.8, 0.8, 0.8},{0.6, 0.8, 0.7, 0.7, 0.7}};

and basically when infection is X below, it doesn't run.

O O X X
O O O O

O O O X
O O O X
O O O O

O O O X X
O O O X X
O O O O O

this is my modification of function 'generate' to debug it:

void generate(const IntegerRaster& infected,
              bool weather, const FloatRaster& weather_coefficient,
              double reproductive_rate)
{
    std::cout << "generate" << std::endl;
    double lambda = reproductive_rate;
    for (unsigned i = 0; i < height; i++) {
        for (unsigned j = 0; j < width; j++) {
            if (infected(i, j) > 0) {
                if (weather)
                    lambda = reproductive_rate * weather_coefficient(i, j); // calculate
                int dispersers_from_cell = 0;
                std::poisson_distribution<int> distribution(lambda);
               
                for (int k = 0; k < infected(i, j); k++) {
                    dispersers_from_cell += distribution(generator);
                }
                dispersers(i, j) = dispersers_from_cell;
                std::cout << "save " << dispersers(i, j) << " " << i << " " << j << std::endl;
            }
            else {
                dispersers(i, j) = 0;
            }
        }
    }
    for (unsigned i = 0; i < height; i++) {
        for (unsigned j = 0; j < width; j++) {
            if (dispersers(i, j) > 0) {std::cout  << "read "<< i << " " << j << std::endl;}
        }
    }
    std::cout << "generate end" << std::endl;
}

Pesticide treatment would be yet another treatment option that would move a percent of the infected and susceptible hosts in a cell, based on the efficacy (parameter) of the treatment to a resistant pool for a specified (maybe with a probability curve) duration (parameter) (e.g. if a fungicide is known to work on ~95% of the population for ~45 days then the function would move 95% of the susceptible and infected hosts in those treated cells to the resistant pool for 45 days). We need to track these resistant moves similar to the way we do for mortality.

Started something here in a rpops branch. We just need to think through the best way to implement this in an easy to use and flexible manner.

Discovered during R cmd check on macOS. Need to add override to get_start and get_end functions in treatment

Steps:

1. Get this repo ready to be tagged for 1.0.0
2. Make the other repos up to date
3. Tag this repo v1.0.0
4. Use v1.0.0 in the other repos
5. Tag other repos v1.0.0
6. Announce release of 1.0.0

Todo list before tagging pops-core v1.0.0:

• Complete issues in this repo with milestone 1.0.0
• Rename this repo to pops-core
• Update name in rpops
• Update name in r.pops.spread
• Update rpops
• Update r.pops.spread
• Specify release policy for core, R, GRASS ("minor mostly for interfaces") - #107
• Update and sync references and authors in all 3 repos - #109
• Remove Makefile
• Update contributing ("branches") - #107

Todo list before v1.0.0 announcement:

• Update to tagged pops-core version in rpops and tag
• Update to tagged pops-core version in r.pops.spread and tag
• Publish r.pops.spread in GRASS GIS addons repo - OSGeo/grass-addons#269
• Update image in pops-intro-grass-notebook - merge ncsu-landscape-dynamics/pops-intro-grass-notebook#1

implement support for multi-host for up to n hosts. This should account for host susceptibility and transmissibility.

Some parts of code need a general mathematical or statistical function which is not in the standard library. Move these function out of the current files into one or more files with general functionality.

After #18 I am getting segfaults when running weekly.

I realized there was a reason I never renamed total_plants to total_hosts. Even in simple SI model without pest resistance, S + I != total_plants because the model includes a dilution effect where number of all (not only host) plants in the cell influences the establishment:

double probability_of_establishment =
                                (double)(susceptible(row, col)) / total_hosts(row, col);

https://github.com/ncsu-landscape-dynamics/PoPS/blob/master/include/pops/simulation.hpp#L419

This is in the disperse function.

On the other hand, the newer movement function assumes S + I == total_hosts. I don't think the limitation/assumption in movement necessitates that the whole model needs to require the same condition as long as it is documented that the movement can only be used when the condition is met.

Add mortality to the library instead of handling it in each interface separately.

There is couple of general utility functions or other definitions in the code. Some wrapping or extending standard library facilities. Move these into as separate file.

Some defs many need to be in a separate file such as an index type which might be needed to at least partially resolve -Wsign-compare from #72.

The ShortLongDispersalKernel class template follows the original naming convention of short and long distance dispersal. Now we call it natural and anthropogenic. The kernel class itself is more general, so there is more options and it is just a question of naming.

ShortLongDispersalKernel(short_kernel, long_kernel, use_long_kernel, percent_short_dispersal)
NaturalAnthropogenicDispersalKernel(natural_kernel, anthropogenic_kernel, use_anthropogenic_kernel, percent_natural_dispersal)
PrimarySecondaryDispersalKernel(primary_kernel, secondary_kernel, use_secondary_kernel, percent_primary_dispersal)
DoubleDispersalKernel(first_kernel, second_kernel, use_second_kernel, percent_first_dispersal)

A separate, yet related, issue is parametrize usage of the kernels. Now it is either first one or both, alternative is supporting not using the first one. We could also have the types (the enum) as a parameter instead of boolean (now called use_long_kernel).

This is mostly about API beauty and it should be easy to keep backwards compatibility, so there is no pressure.

Add transmission between two simulations one for a vector and one for a host. Also, add the ability to track 2 separate pests or pathogens.

To reduce overall stochasticity of the model, optionally remove stochasticity from the Simulation::generate() function.

We miss instructions for core developers on where to put fixes. master? development (now 11 commits behind master)? fix/... branch (from what)? pull request (against what)?

Example, I have the fix for #13, but it is not clear where to put it.

Relates to #2 (originally meant for external contributors).

Functions such as treatment_app_enum_from_string() in ncsu-landscape-dynamics/r.pops.spread should go into the C++ library itself similarly to kernel_type_from_string() which is already there.

Add simple file reading to fill in the Config.

All we need is something like:

key1: value1
key2: value2

i.e., no nesting.

Rename this repo and project from PoPS to pops-core. The C++ headers should be still in pops directory and the C++ namespace still pops. The -core means something along the lines of lib or main part, so no reason to add it to header location or namespace. It is core in the relation to rest of the PoPS project which should be the one using the name PoPS.

This is needed for C/C++ for printing debug messages (in r.pops.spread).

Build also with Clang, not only GCC, in the CI.

Motivation: #111, #112, -Wall result in different warnings being enabled, rpops is build with clang on macOS.

Wrap all dispersal kernel code into a generic API (function or class) so that no complexity of dispersal kernel code is in the Simulation itself, but instead, it is handled separately and a customizable way. This should remove all (or most) of the parameters related to kernels from the Simulation.disperse() function resulting in simpler Simulation calls and possible higher customization of kernels.

However, the possibility to modify kernel parameters during the simulation will be preserved because the whole kernel function/object can be replaced or modified when calling the Simulation.disperse() function.