Giter Club home page Giter Club logo

daisiemainland's Introduction

Hi ๐Ÿ‘‹

I'm Josh, an evolutionary biologist ๐Ÿงฌ ๐Ÿธ, turned research software engineer ๐Ÿ’ป.

I'm currently working at the The London School of Hygiene & Tropical Medicine (LSHTM) on the Epiverse initiative, and finishing my PhD at the University of Groningen. My PhD is on the topic of developing novel phylogenetic models to understand macroevolutionary patterns on islands โ›ฐ๏ธ.

  • ๐Ÿ”ญ Iโ€™m currently working on: Epiverse by day, macroevolution and island biogeography by night.
  • ๐ŸŒฑ Iโ€™m currently learning: most things, but mainly epidemiology of infectious diseases.
  • ๐Ÿ‘ฏ Iโ€™m looking to collaborate on: interesting research questions at the intersection of evolutionary biology and epidemiology.
  • ๐Ÿค” Iโ€™m looking for help with: not sure ... but fun collaborators are always welcome ๐Ÿ˜ƒ
  • ๐Ÿ’ฌ Ask me about: anything!
  • ๐Ÿ“ซ How to reach me: [email protected]
  • ๐Ÿ˜„ Pronouns: he/him
  • โšก Fun fact: I'm a fan of all things Monocle

Email Twitter

snake gif

daisiemainland's People

Contributors

joshwlambert avatar neves-p avatar richelbilderbeek avatar

Stargazers

 avatar

Watchers

 avatar

Forkers

neves-p

daisiemainland's Issues

plot_daisie_mainland_data: reverse time axis

This Issue came from #62.

From Josh's comments and code, I added this test:

test_that("Issue 64: reverse axes", {

  # The Endemic_singleton_MaxAge island species are plotted near
  # the edge of the plot but they should have the longest line.
  # This makes me think that the plot_daisie_mainland_data function
  # should plot "Time before present" on the x-axis.
  set.seed(
    1,
    kind = "Mersenne-Twister",
    normal.kind = "Inversion",
    sample.kind = "Rejection"
  )

  daisie_mainland_data <- DAISIEmainland::sim_island_with_mainland(
    total_time = 1,
    m = 100,
    island_pars = c(1.0, 0.5, 10, 0.1, 0.5),
    mainland_ex = 2,
    mainland_sample_prob = 1,
    mainland_sample_type = "complete",
    replicates = 1,
    verbose = FALSE
  )

  DAISIEmainland::plot_daisie_mainland_data(
    daisie_mainland_data = daisie_mainland_data,
    replicate_index = 1
  )
})

Suggest: uniform 'island_replicates' datatype

Dear DAISIEmainland maintainers,

Thanks for DAISIEmainland as well as its increasing clarity in its data types.

The data type called island_replicates, however, has two different layouts, hence I will recommend to change one of the two.

To start: format_to_daisie_data is a function that works on a island_replicates, as is created in the sim_island_with_mainland function:

sim_island_with_mainland <- function(
  #...
) {
  # ...
  island_replicates <- list()

  for (rep in seq_len(replicates)) {
    # ...
    island_replicates[[rep]] <- full_list
  }

  # ...
  daisie_data <- format_to_daisie_data(
    island_replicates = island_replicates,
    # ...
  )

  # ...
}

Zooming on on format_to_daisie_data, the island_replicates are modified and then used in format_to_daisie_data_core with the same name:

format_to_daisie_data <- function(island_replicates,
                                  total_time,
                                  m) {
  # Good: check input
  DAISIEmainland::check_island_replicates(island_replicates)

  
  ideal_island_replicates <- list()
  empirical_island_replicates <- list()
  for (i in seq_along(island_replicates)) {
    # ...
  }

  # here, 'island_replicates' is of a different type!
  # Suggest: emperical_or_ideal_island_replicates (yes, it is an awkwardly long name)
  ideal_islands <- format_to_daisie_data_core(
    island_replicates = ideal_island_replicates, 
    total_time = total_time,
    m = m)

  # here, 'island_replicates' is of a different type!
  # Suggest: emperical_or_ideal_island_replicates (yes, it is an awkwardly long name)
  empirical_islands <- format_to_daisie_data_core(
    island_replicates = empirical_island_replicates,
    total_time = total_time,
    m = m)

  #...
  daisie_data
}

I suggest that format_to_daisie_data_core uses a emperical_or_ideal_island_replicates (yes, it is an awkwardly long name) as its input, to ensure to have uniform data types (#38, #37).

@joshwlambert what do you think?

The awkward name can be prevented by refactoring sim_island_with_mainland. For examples, making island_replicates a list of island would be a clean step in the right direction.

CRAN disallows installing GitHub repos from vignettes

Dear DAISIEmainland maintainer,

Thanks for DAISIEmainland and a vignette that demonstrates part of its workings!

I did spot a CRAN violation in the vignette and I suggest to fix it (which is easy). In the vignette it states:

```{r install dependencies}
remotes::install_github("rsetienne/DAISIE")
remotes::install_github("thijsjanzen/nLTT")
remotes::install_github("joshwlambert/DAISIEmainland")
```

I do enjoy the helpfulness towards the users in this, but it is against CRAN policies to modify the local environment of the user (in this case, the installed packages on his/her harddisk), probably for good reasons.

I suggest to move these lines to the README.md instead.

Clean up within-function checks when done

To make sure the check_ functions work, as well as generate new test cases, I added within-function checks on the return type. The return type should not be checked within the function, as this needlessly slows down the package.

Remove the within-function checks when done.

Create `plot_daisie_data`

Currently, we can plot the mainland and the island, i.e. data types mainland and island. However, the daisie_data cannot be plotted yet, which would be useful to verify the format_to_daisie_data function works nice.

It is easy to get a daisy_data from a daisie_mainland_data:

  daisie_mainland_data <- sim_island_with_mainland(
    total_time = 1,
    m = 10,
    island_pars = c(1, 1, 10, 0.1, 1),
    mainland_ex = 1,
    mainland_sample_prob = 1,
    mainland_sample_type = "undiscovered",
    replicates = 10,
    verbose = FALSE
  )
  ideal_daisie_data <- daisie_mainland_data$ideal_multi_daisie_data[[1]] # a daisie_data
  empirical_daisie_data <- daisie_mainland_data$empirical_multi_daisie_data[[1]] # a daisie_data

Plot each of these using plot_daisie_data

Example in `DAISIEmainland` doc does not work

Dear DAISIEmainland maintainer,

Thanks for DAISIEmainland and having a DAISIEmainland main documentation page (as can be reached by `?DAISIEmainland) with an example.

I do see the example does not work and gives the error Error in eval(expr, envir = envir, enclos = NULL) : argument "mainland_sample_type" is missing, with no default. The reason for this is clear: the example is put into a \dontrun{} (hence, unchecked) section; here I copied from DAISIEmainland.R:

#' @examples
#' \dontrun{
#' # simulate data for 1000 islands
#' replicates <- 1000
#' island <- sim_island_with_mainland(
#' total_time = 1,
#' m = 100,
#' island_pars = c(1, 1, 10, 0.1, 1),
#' mainland_ex = 1,
#' mainland_sample_prob = 1,
#' replicates = replicates,
#' verbose = FALSE
#' )
#'}

I suggest to both fix the example and to remove \dontrun{} (because these are unchecked and thus possibly wrong) sections from the documentation.

Suggest: let vignette do actual calculations

Dear DAISIEmainland maintainer,

Thanks for writing DAISIEmainland and including a vignette to demonstrate the working of your package.

However, upon closer look, I see all code blocks have eval=FALSE, so they do not actually run. The reason is obvious: the calculation will take very long (among others, there are 1000 replicates).

I suggest to make the vignette do actual work, yet for more modest settings (e.g. 3 replicates), so a user can be convinced of the awesomeness of DAISIEmainland.

BTW, I do volunteer to do so, e.g. on the richel branch, then sending a Pull Request :-)

Suggest: use `create_test_island_spec`

Dear DAISIEmainland maintainer,

Thanks for DAISIEmainland and its thorough testing! One of my favorite functions in that regard is create_test_island_spec, with examples for 43 scenarios! That seems like a solid foundation for a solid R package!

What is unexpected, however, is that it is never actually used, e.g. in plotting functions. Also, the tests for some scenarios is exactly the same:

test_that("create_test_island_spec produces correct output for scenario 42", {
  island_spec <- create_test_island_spec(island_scenario = 42)
  expect_true(is.data.frame(island_spec))
  expect_true(ncol(island_spec) == 7)
  expect_true(nrow(island_spec) == 2)
})

test_that("create_test_island_spec produces correct output for scenario 43", {
  island_spec <- create_test_island_spec(island_scenario = 43)
  expect_true(is.data.frame(island_spec))
  expect_true(ncol(island_spec) == 7)
  expect_true(nrow(island_spec) == 2)
})

In other words, the different scenarios are untested to be different.

From this line of reasoning, the create_test_island_spec appears to have underused potential. I do predict this function had their use for e.g. a function plotting the ontology (#11), but as that plotting functionality did not get in, create_test_island_spec was left close to empty handed.

I do see that some scenarios of create_test_island_spec are used (see below), yet nowhere I see a test with e.g. scenario 42 and 43 give different results.

Screenshot from 2021-11-01 11-32-19

As an observant user, I hope I can see all create_test_island_spec scenario's shine, it would definitely be helpful to be sure DAISIEmainland works awesomely!

Remove link to Tidyverse coding standard?

I've added some code that ends with:

some_function_name <- function(...) {
  # ...

  # See https://style.tidyverse.org/functions.html#return
  invisible(return_type)
}

@joshwlambert do you agree to follow the Tidyverse recommendations here?

If yes, I will happily remove the comments linking to the Tidyverse style guide :-)

Suggest: add supplemenary materials as PDF

Dear DAISIEmainland maintainer.

Thanks for DAISIEmainland and its excellent article-in-preparation. Especially the supplementary materials, with the 43 scenarios are useful for the package.

Could these (i.e. the PDF pages that describe the 43 scenarios) already (i.e. before publication) be added to the package, e.g. in inst/extdata?

Would be awesome and allow me/users to use scenario's in their questions :-)

Function housekeeping

It's create to have to functions to help convert between things in a readable way. Please move all of these functions into a file called sim_utils. A plot_utils file already exists so if any of them are exclusively used for plotting please put them in plot_utils instead. If they are not used for in the simulation or plotting, but rather the testing, please create a file called test_utils and put them there.

purr and tidyverse

Currently purrr is called but not stated as a dependency. Also, I am not a fan of having tidyverse dependencies (exception being ggplot2), please only use when necessary over using base R functions.

Use time before present + plot recolonisations

The Endemic_singleton_MaxAge island species are plotted near the edge of the plot but they should have the longest line. This makes me think that the plot_daisie_mainland_data function should plot "Time before present" on the x-axis. @richelbilderbeek would this be possible for you to implement?

Feature request: plot the ontology

Dear DAISIEmainland maintainer,

Thanks for DAISIEmainland and its many plotting functions!

The plotting function that is missing however, is one to plot the ontology of species, i.e. that produces pictures such as this:

Screenshot from 2021-11-01 10-29-40

I know it is a hard problem to produce beautiful plots, yet from the thoroughness of DAISIEmainland I predict attempts at plotting this has been made. If I am correct, I predict this functionality has been removed as it did not always results in beautiful plots. If so, I'd prefer to be able to see 'ugly' plot over nothing at all, maybe even from a different package (DAISIEmainlandplot seems a pretty name to me :-) ).

What is the status and what are the plans for this plotting functionality?

Suggest: remove or activate skipped tests

Dear DAISIEmainland maintainer,

Thanks for DAISIEmainland and -among others- the proof it works correct by writing tests to achieve an awesome code coverage! However, when I search the package for the word skip I find 13 instances where a test is skipped. It makes me wonder if those tests should be actually run or be removed?

I suggest to either remove the skip or remove the test, to remove this confusion.

I do have an idea about some skips why these are there: these take too long (and we know devs have an attention span of 10 secs)! Tricks I use to workaround this are (1) only run those tests on a Continuous Intrgration service (example from babette here), or (2) simplify the test (e.g. use 3 instead of 1000 replicates).

Write function to create `ideal_island_tbl` and `emperical_island_tbl` from `island_tbl`

Currently, plot_island_tbl works on an island_table, where the island_table does not yet separate for an omniscient/emperical observer. As far as I understand (see email exchange below), this is possible.

Email

From me: @richelbilderbeek

I was working on plot_island.

I feel I miss quite some information: where are the empirical and ideal data used by plot_island_tbl? sim_island used to return both. What do I overlook here?

From @joshwlambert:

The island_tbl is lower level than the ideal and empirical data. island_tbl is produced by sim_island and then the island_tbl is passed to create_daisie_data where the ideal and empirical data is created [...]

I just thought about the issue and I think it's not trivial. Ideally we would want to be able to plot the ideal and empirical data, but this is not available in the island_tbl and we want to use the information from island_tbl as this contains more information than the ideal and empirical data. Therefore I suggest we write a new function that is specifically designed to take the island_tbl and create an ideal_island_tbl and empirical_island_tbl that can be used specifically for plotting. What do you think?

Feature request: sim_island_with_mainland for t = 0

Dear DAISIEmainland maintainer,

Thanks for DAISIEmainland and its many tests to ensure its correctness! It is great to see that all input variables and their constraints are checked early in the calculation!

I feel however, that one constraint can be loosened: the minimal simulation time. When using sim_island_with_mainland, the total_time must be bigger than zero. However, I feel it makes sense to create the data without anything happening.

In code, I suggest this test to pass:

test_that("sim_island_with_mainland with 0.0 time", {
  expect_silent(
    sim_island_with_mainland(
      total_time = 0.0, # Nothing happened yet
      m = 2, # Irrelevant
      island_pars = c(1, 1, 10, 0.1, 1), # Irrelevant
      mainland_ex = 1, # Irrelevant
      mainland_sample_prob = 1, # Irrelevant
      mainland_sample_type = "undiscovered", # Irrelevant
      replicates = 1 # Irrelevant
    )
  )
})

I already checked that if I changed testit::assert(total_time > 0) to testit::assert(total_time >= 0), the code just runs fine and the tests pass.

I hope you'll loose up this constraint, so empty islands and mainlands can be created. Or, to prevent me writing the code below :-) :

sim_island_with_mainland(
  total_time = 0.0000000000000001, # Looks silly! Sure, .Machine$double.xmin looks cooler :-)
  # ...
)

Suggest: doc and test `format_to_daisie_data`

Dear DAISIEmainland maintainer,

Thanks for DAISIEmainland and its many tested. It is great to see other people trying hard to generate simulations that are actually correct!

When checking the inner working of DAISIEmainland, however, I feel that sim_island_with_mainland is not one, but two black boxes, here I show what I mean in code:

sim_island_with_mainland <- function(
  # ...
) {
  # ...

  island_replicates <- list()

  # Black box 1: things with island replicates

  # Black box 2; format to DAISIE data
  island_replicates <- format_to_daisie_data(
    island_replicates = island_replicates,
    # ...  
  )

}

Now, it is acceptable that internal code is a bit of a black box. What is unexpected to me, however, is the lack of both tests and documentation of format_to_daisie_data.

Would it be possible to add documentation and/or tests for format_to_daisie_data to a precise reader such as I can get a feeling of what is going on?

Would be awesome!

Suggest: show the resulting errors in the vignette

Dear DAISIEmainland maintainer,

Thanks for DAISIEmainland and having a vignette to demonstrate what it does for a relatively simple parameter set.

Unexpectedly, in the end, when the inference error is calculated (see vignette code below) ...

```{r Calculate error}
errors <- DAISIEmainland::calc_error(
  daisie_data = island,
  ideal_ml = ideal_ml,
  empirical_ml = empirical_ml)
```

... the resulting errors are not displayed on screen, nor interpreted.

I suggest to show the resulting errors and give an intepretation. Sure, maybe the seed needs to be changed to a more interesting-yet-simple scenario :-)

Mainland extinction correct?

Dear DAISIEmainland maintainer,

Thanks for DAISIEmainland and its well-written and well-tested code! I've already mentioned that I missed a function to plot mainland and island, so -up till now- I wrote a function to plot the mainland (it is on branch richel). Within these plots, I do see things I do not understand. Please correct me.

So, here is my first test, in test-plot_mainland.R:

test_that("minimal example use", {
  set.seed(
    1,
    kind = "Mersenne-Twister",
    normal.kind = "Inversion",
    sample.kind = "Rejection"
  )
  mainland <- DAISIEmainland:::sim_mainland(
    total_time = 1,
    m = 2,
    mainland_ex = 1)

  plot_mainland(mainland)
})

This is the code and plot:

Screenshot from 2021-11-24 21-19-12

This is great! The number of species in all clades in nicely preserved (I first thought this was a mistake, but this 'duo pattern' always happens when there is only 1 clade).

Now to a more interesting example, with only a different seed and extinction rate:

test_that("interesting example", {
  set.seed(
    4,
    kind = "Mersenne-Twister",
    normal.kind = "Inversion",
    sample.kind = "Rejection"
  )
  mainland <- DAISIEmainland:::sim_mainland(
    total_time = 1,
    m = 4,
    mainland_ex = 1)

  plot_mainland(mainland)
})

Screenshot from 2021-11-24 21-23-20

Now, this is something I'd like to be checked upon:

At t = 0.04290015 (see mainland printout below), in clades 3 and 4, the same extinction seems to happen and this results in a nice duo in clade 4. Should it be possible that two species from different clades go extinct at exactly the same time?

The answer: yes! When species 15 (in clade 3) goes extinct, it lets the ancestral species in clade 4 speciate!

> mainland
[[1]]
  spec_id main_anc_id spec_type branch_code  branch_t spec_origin_t spec_ex_t
1       1           1         E           A       NaN     0.0000000 0.3282561
2      11           1         E          AA 0.3282561     0.3282561 0.4520710
3      12           1         E          AB 0.3282561     0.3282561 0.4520710
4      13           1         E         ABA 0.4520710     0.4520710 0.7088819
5      14           1         E         ABB 0.4520710     0.4520710 0.8010316
6      15           1         E        ABAA 0.7088819     0.7088819 0.7237706
7      16           1         C        ABAB 0.7088819     0.7088819 1.0000000

[[2]]
  spec_id main_anc_id spec_type branch_code  branch_t spec_origin_t spec_ex_t
1       2           2         E           A       NaN     0.0000000 0.2562369
2       9           2         E          AA 0.2562369     0.2562369 0.7237706
3      10           2         E          AB 0.2562369     0.2562369 0.3282561
4      17           2         E         AAA 0.7237706     0.7237706 0.8010316
5      18           2         C         AAB 0.7237706     0.7237706 1.0000000
6      19           2         C        AAAA 0.8010316     0.8010316 1.0000000
7      20           2         C        AAAB 0.8010316     0.8010316 1.0000000

[[3]]
  spec_id main_anc_id spec_type branch_code branch_t spec_origin_t  spec_ex_t
1       3           3         E           A      NaN             0 0.04290015

[[4]]
  spec_id main_anc_id spec_type branch_code   branch_t spec_origin_t  spec_ex_t
1       4           4         E           A        NaN    0.00000000 0.04290015
2       5           4         E          AA 0.04290015    0.04290015 0.24356191
3       6           4         E          AB 0.04290015    0.04290015 0.24356191
4       7           4         E         ABA 0.24356191    0.24356191 0.25623688
5       8           4         E         ABB 0.24356191    0.24356191 0.70888185```

Add `plot_daisie_mainland_data`

Depends on:

From an email between me and Josh, we concluded it would be good to get the code below to work:

  set.seed(
    2,
    kind = "Mersenne-Twister",
    normal.kind = "Inversion",
    sample.kind = "Rejection"
  )
  daisie_mainland_data <- sim_island_with_mainland(
    total_time = 1,
    m = 10,
    island_pars = c(1, 1, 10, 0.1, 1),
    mainland_ex = 1,
    mainland_sample_prob = 1,
    mainland_sample_type = "undiscovered",
    replicates = 10,
    verbose = FALSE
  )

  plot_daisie_mainland_data(daisie_mainland_data, replicate_index = 1) # Probably calls the next two functions :-)
  plot_empirical_multi_daisie_data(daisie_mainland_data$empirical_multi_daisie_data, replicate_index = 1)
  plot_ideal_multi_daisie_data(daisie_mainland_data$ideal_multi_daisie_data, replicate_index = 1)

In this Issue, I do so.

Plot all recolonisations in 'plot_daisie_mainland_data'

From Issue 62 here I quote:

[...] In this example the ideal data should plot a recolonisation, but only plots one of the species from the all_colonisations.

set.seed(
  1,
  kind = "Mersenne-Twister",
  normal.kind = "Inversion",
  sample.kind = "Rejection"
)

daisie_mainland_data <- DAISIEmainland::sim_island_with_mainland(
  total_time = 1,
  m = 50,
  island_pars = c(1.0, 0.5, 10, 0.1, 0.5),
  mainland_ex = 2,
  mainland_sample_prob = 1,
  mainland_sample_type = "complete",
  replicates = 1,
  verbose = FALSE
)

DAISIEmainland::plot_daisie_mainland_data(
  daisie_mainland_data = daisie_mainland_data,
  replicate_index = 1
)

image

Feature request: mainland with 1 clade

Dear DAISIEmainland maintainer,

Thanks for DAISIEmainland and its many tests to ensure its correctness!

Here I suggest to loosen up a contraint, as I feel (and tested informally) that this is not a real constraint.
Currently, m (the number of mainland clades) has to be more than 1 (i.e. at least 2).
I suggest to allow m to be more than zero (i.e. at least 1), as this nicely matches the examples as given in the (excellent!) PDF in inst/extdata.

In code, I suggest to make the following test pass:

test_that("sim_island_with_mainland with 1 mainland clade", {
  expect_silent(
    sim_island_with_mainland(
      total_time = 1, # Irrelevant
      m = 1, # Number of mainland clades
      island_pars = c(1, 1, 10, 0.1, 1), # Irrelevant
      mainland_ex = 1, # Irrelevant
      mainland_sample_prob = 1, # Irrelevant
      mainland_sample_type = "undiscovered", # Irrelevant
      replicates = 1 # Irrelevant
    )
  )
})

I have checked in the code if replacing testit::assert(m > 1) by testit::assert(m >= 1) gave any problems, but no, that worked great!

I'd be happy to do the replacement myself, but I want to be sure I do not overlook something here :-)

`mainland_sample_type` can be nonsense

Dear DAISIEmainland maintainer,

Thanks for DAISIEmainland and its many tests to ensure its correctness.

In test-sim_island.R, I was happy to find the many tests to confirm that a user receives a nice error message, shown here for reference:

test_that("sim_island fails with incorrect input", {
  expect_error(sim_island(
    total_time = "nonsense",
    island_pars = c(1, 1, 10, 1, 1),
    mainland_clade = create_test_mainland_clade(mainland_scenario = 1),
    mainland_sample_prob = 1)
  )

  expect_error(sim_island(
    total_time = 1,
    island_pars = "nonsense",
    mainland_clade = create_test_mainland_clade(mainland_scenario = 1),
    mainland_sample_prob = 1)
  )

  expect_error(sim_island(
    total_time = 1,
    island_pars = c("nonsense", 1, 10, 1, 1),
    mainland_clade = create_test_mainland_clade(mainland_scenario = 1),
    mainland_sample_prob = 1)
  )

  expect_error(sim_island(
    total_time = 1,
    island_pars = c(1, "nonsense", 10, 1, 1),
    mainland_clade = create_test_mainland_clade(mainland_scenario = 1),
    mainland_sample_prob = 1)
  )

  expect_error(sim_island(
    total_time = 1,
    island_pars = c(1, 1, "nonsense", 1, 1),
    mainland_clade = create_test_mainland_clade(mainland_scenario = 1),
    mainland_sample_prob = 1)
  )

  expect_error(sim_island(
    total_time = 1,
    island_pars = c(1, 1, 10, "nonsense", 1),
    mainland_clade = create_test_mainland_clade(mainland_scenario = 1),
    mainland_sample_prob = 1)
  )

  expect_error(sim_island(
    total_time = 1,
    island_pars = c(1, 1, 10, 1, "nonsense"),
    mainland_clade = create_test_mainland_clade(mainland_scenario = 1),
    mainland_sample_prob = 1)
  )

  expect_error(sim_island(
    total_time = 1,
    island_pars = c(1, 1, 10, 1, 1),
    mainland_clade = "nonsense",
    mainland_sample_prob = 1)
  )

  expect_error(sim_island(
    total_time = 1,
    island_pars = c(1, 1, 10, 1, 1),
    mainland_clade = create_test_mainland_clade(mainland_scenario = 1),
    mainland_sample_prob = "nonsense")
  )
})

These tests check all the input arguments of sim_island to be valid.

Except for one: mainland_sample_type, is missing.

I suggest to add the following test (I have confiremed it fails now) to ensure that sim_island properly checks all inputs:

  expect_error(sim_island(
    total_time = 1,
    island_pars = c(1, 1, 10, 1, 1),
    mainland_clade = create_test_mainland_clade(mainland_scenario = 1),
    mainland_sample_prob = 1,
    mainland_sample_type = "nonsense")
  )

Note that I found out when I used mainland_sample_type = "ideal", which is an invalid input (complete is the value I should have used here).

Thanks for considering my suggestion!

Suggest: document prerequisites

Dear DAISIEmainland maintainer,

Thanks for DAISIEmainland and -among others- the fine 'Installation' section in the README. The describe installation method, however, is incomplete, as the CRAN versions of DAISIE and nLTT are not new enough for DAISIEmainland

I suggest to also document the installation of these packages, i.e.:

remotes::install_github("rsetienne/DAISIE")
remotes::install_github("thijsjanzen/nLTT")

In that way, the users do not need to scout for the origin of these packages (sure, these origins are in DESCRIPTION as well, but -among others- my mom would not know that)

I do volunteer to do so, via a Pull Request from richel to develop

Note to self: `sim_island` always returns an uninteresting result?

Will continue on this tomorrow...

test_that("use", {
  set.seed(
    1,
    kind = "Mersenne-Twister",
    normal.kind = "Inversion",
    sample.kind = "Rejection"
  )
  mainland <- DAISIEmainland:::sim_mainland(
    total_time = 1,
    m = 10,
    mainland_ex = 2)
  plot_mainland(mainland = mainland)

  count_extant_mainland_species <- function(mainland) {
    purrr::map_dbl(mainland, function(x) { return(sum(x$spec_ex_t == 1.0)) } )
  }
  clade_with_most_species <- which(
    count_extant_mainland_species(mainland) ==
      max(count_extant_mainland_species(mainland))
  )

  mainland_clade <- mainland[[clade_with_most_species]]
  cladogenesis_rate <- 100.0
  extinction_rate <- 100.0
  carrying_capacity <- 100.0
  immigration_rate <- 100.0
  anagenesis_rate <- 100.0

  island_pars <- c(
    cladogenesis_rate,
    extinction_rate,
    carrying_capacity,
    immigration_rate,
    anagenesis_rate
  )
  mainland_sample_type <- "unsampled"
  mainland_sample_type <- "undiscovered"
  mainland_sample_type <- "complete"
  testit::assert(mainland_sample_type == "unsampled" ||
                   mainland_sample_type == "undiscovered" ||
                   mainland_sample_type == "complete")

  island <- sim_island(
    total_time = 100,
    island_pars = island_pars,
    mainland_clade = mainland_clade,
    mainland_sample_prob = 1,
    mainland_sample_type = mainland_sample_type)
  island
  plot_island(island)
})

Add data documentation

Old documentation:

#' Parameter space for the analysis of the DAISIE mainland extinction model.
#'
#' A dataset containing the parameter sets (rows) for the simulation of the
#' DAISIE mainland extinction model for the analysis of the error inferred by
#' DAISIE's maximum likelihood model.
#'
#' @format A data frame with 768 rows and 10 variables:
#' \describe{
#'   \item{total_time}{Duration of simulation (million years)}
#'   \item{m}{Number of species on the mainland}
#'   \item{island_clado}{Rate of cladogenesis on the island}
#'   \item{island_ex}{Rate of extinction on the island}
#'   \item{island_k}{Carrying capacity for each island clade}
#'   \item{island_immig}{Rate of immigration on the island}
#'   \item{island_ana}{Rate of anagenesis on the island}
#'   \item{mainland_ex}{Rate of extinction on the mainland}
#'   \item{mainland_sample_prob}{Probability of a mainland species being
#'     sampled at the end of the simulation if it is extant}
#'   \item{replicates}{Number or island replicates}
#' }
"param_space"

stac 1 and stac 4 identical?

Dear DAISIEmainland maintainers,

Thanks for DAISIEmainland and its fine documention!

When I take a look at the sim_island_with_mainland documentation, however, ...

?sim_island_with_mainland

I see that the descriptions for stac == 1 and stac == 4 are identical (both are Non_endemic_MaxAge):

Screenshot from 2021-11-29 17-58-59

I assume there is a difference and that the documentation had just a copy-paste error.

I hope it will be fixed so that the doc of DAISIEmainland is even awesomer!

Suggest: export `sim_mainland`

Dear DAISIEmainland maintainer,

Thanks for DAISIEmainland and its broad range of functionality it provides.

I developed a simple function called plot_mainland (which I'll put in a Pull Request to develop soon) to plot the simulated mainland clades. Its documentation looks like this:

#' mainland <- DAISIEmainland:::sim_mainland(
#'   # ... 
#' )
#'
#' plot_mainland(mainland)

Note the use of the triple colons ::: when calling sim_mainland, as sim_mainland is not an exported function.

I suggest to export sim_mainland to make that documentation prettier. Or, when reasoned from another perspective: I suggest to export sim_mainland as it is apparently important enough to make it into the documentation.

And hey, I'd be happy to do so myself in a Pull Request :-)

Write read_results function

Write a read function for results instead of having plots load from a file. This will then allow the plots to use data locally in the work environment or saved in an external directory.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.