openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
717 stars 38 forks source link

[REVIEW]: DAISIEmainland: an R package for simulating an island-mainland system for macroevolution on islands #4491

Closed editorialbot closed 2 years ago

editorialbot commented 2 years ago

Submitting author: !--author-handle-->@joshwlambert<!--end-author-handle-- (Joshua Lambert) Repository: https://github.com/joshwlambert/DAISIEmainland Branch with paper.md (empty if default branch): joss_submission Version: v1.0.0 Editor: !--editor-->@Bisaloo<!--end-editor-- Reviewers: @alexskeels, @vboussange Archive: Pending

Status

status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/6913e20fc41640a53c0c8ca75d9af950"><img src="https://joss.theoj.org/papers/6913e20fc41640a53c0c8ca75d9af950/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/6913e20fc41640a53c0c8ca75d9af950/status.svg)](https://joss.theoj.org/papers/6913e20fc41640a53c0c8ca75d9af950)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@alexskeels & @vboussange, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @Bisaloo know.

Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest

Checklists

📝 Checklist for @vboussange

📝 Checklist for @alexskeels

editorialbot commented 2 years ago

Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.

For a list of things I can do to help you, just type:

@editorialbot commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@editorialbot generate pdf
editorialbot commented 2 years ago
Software report:

github.com/AlDanial/cloc v 1.88  T=0.24 s (920.8 files/s, 106951.7 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
R                              159           1098           1919          14636
HTML                             9            157              0           2322
CSS                             11             93             88           1280
JavaScript                      15            176            152           1012
Markdown                         9            314              0            973
Rmd                              8            172            433            339
SVG                              1              0              0            288
TeX                              2             12              0            152
YAML                             3             18              6            106
Bourne Shell                     3              3             23             11
JSON                             2              0              0              2
-------------------------------------------------------------------------------
SUM:                           222           2043           2621          21121
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 2 years ago

Wordcount for paper.md is 1314

editorialbot commented 2 years ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/0021-9991(76)90041-3 is OK
- 10.1017/S0305004100033193 is OK
- 10.1038/s41586-020-2022-5 is OK
- 10.1146/annurev.physchem.58.032806.104637 is OK
- 10.1021/j100540a008 is OK
- 10.1111/ele.12461 is OK
- 10.1111/jeb.12139 is OK
- 10.1101/2022.01.13.476210 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 2 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

Bisaloo commented 2 years ago

:wave: :wave: :wave: @joshwlambert, @alexskeels, @vboussange, this is the review thread for the paper. All of our communications will happen here from now on.

As a reviewer, the first step is to create a checklist for your review by entering

@editorialbot generate my checklist

as the top of a new comment in this thread.

These checklists contain the JOSS requirements. As you go over the submission, please check any items that you feel have been satisfied. The first comment in this thread also contains links to the JOSS reviewer guidelines.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention openjournals/joss-reviews#4491 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for reviews to be completed within about 2-4 weeks. Please let me know if any of you require some more time. We can also use EditorialBot (our bot) to set automatic reminders if you know you'll be away for a known period of time.

Please feel free to ping me (@bisaloo) if you have any questions/concerns.

vboussange commented 2 years ago

Review checklist for @vboussange

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

vboussange commented 2 years ago

Hi everyone 👋 I am posting here my comments for the first round of review.

General comments

Reviewer summary

DAISIEmainland is a package that allows to simulate the evolutionary history of an island community, bringing more realism to the DAISIE model by simulating the evolutionary history of the mainland community.

General comment

The package is well constructed, with a nice documentation, complete unit-tests, and community guidelines. However, I am not convinced of the usefulness of the package for the community, as it is presented right now. It seems that this package has been specifically designed to lead the investigation of the autor's work "The effect of mainland dynamics on data and parameter estimates in island biogeography", available as a preprint at https://www.biorxiv.org/content/10.1101/2022.01.13.476210v2. While I do not question the relevance of this study, I do not see the point of publishing the corresponding code as a standalone package. Even if the authors would less emphasize on the capacity of DAISIEmainland to test the DAISIE inference performance, and rather put emphasis on the capacity of DAISIEmainland to obtain more realistic evolutionary history simulations, I think that it would benefit the user to integrate those changes in the main DAISIE package, as most of the syntax is the same. Overall, my opinion is that the code of DAISIEmainland could be a nice contribution to bring more functionalities to DAISIE, but it does not justify a standalone package.

Specific comments

Functionality

Installation

remotes::install_github("rsetienne/DAISIE")

Does unfortunately not work for me, I get an error with gfortran. The error is documented here but the fix did not work for me. It would be nice to indicate this potential fix to the user in the installation page.

Documentation

Simulation algorithm

Species that do not go extinct are give a spec_ex_t equal to the total time of the simulation.

The algorithm checks whether any changes have occured on the mainland since the last time step and if so, the system is updated and the returned to the time at which the mainland last changed.

Not clear

where r_j are the rates.

Breaking down the sentence leads to confusion, would be nice to rephrase this sentence.

$X = \lambda ...$

I think that you are here confusing about the random variable which serves as time steps and the probability density. It would probably make more sense to denote this random variable $\Delta t$

Simulation data visualisation

An island with a single endemic species which whose colonisation time is older than the island age so it is considered an island-age colonisation. The uncertainty in colonisation time is handled in an equivalent way to the non-endemic island-age colonisation.

An island with an recolonisation of the same mainland species after it has colonised and speciated (either via cladogenesis or anagenesis on the island). The colonists that are from the same ancestral mainland species but the island species is now endemic are show by ‘Colonist after anagenesis’ and ‘Colonist after cladogenesis.’

Inference performance

simualation algorithm

Paper

Summary

1) simulating the evolutionary history on island species,

Not sure what is meant by that.

Statement of need

the performance and robustness of this island biogeography inference model is unknown when its assumptions are violated under biologically realistic scenarios What is meant by "performance" here? "robustness"? I would simply delete this sentence.

A central assumption of the DAISIE likelihood model

Not sure what is a likelihood model. Overall, l36-37 could be better formulated.

The DAISIEmainland package has been applied to test the ... which provides a suite of phylogenetic likelihood inference models for island biogeography.

This should not belong to the statement of need. It rather describes the package, and I think that it would rather fit in the summary.

Bisaloo commented 2 years ago

Hi @alexskeels :wave:, do you expect to be able to complete your first round of review soon? It's okay if you need more time but having a tentative timeline would be helpful :relaxed:

alexskeels commented 2 years ago

Review checklist for @alexskeels

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

alexskeels commented 2 years ago

Hello @Bisaloo, @joshwlambert, @vboussange. I am very sorry for my lateness - i've been away on holidays.

A few technical issues prevented a complete review of the package. I ran into the folllowing error in vignette 4 running the ML inference: "Error in DAISIE_ode_cs(initprobs, tvec, parsvec, atol, rtol, method, runmod = "daisie_runmod") : cpp_daisie_cs_runmod: too many steps"

and in vignettes 5 for summarising the results: DAISIEmainland::calc_num_spec doesn’t run “Error in stacs[[j]] : subscript out of bounds” DAISIEmainland::calc_num_col “Error in stacs[[j]] : subscript out of bounds”

I am running R version 4.1.2 on a windows machine if thats helpful at all. These should probably be addressed before assessing the checklist "Functionality" and "Performance".

One further general comment: The model simulates the mainland clade with an extinction rate and then speciation immediately follows extinction. How come the mainland pool has to remain a fixed size? Can it evolve under a BD model which might better represent flcutuating clade dyanmics on the mainland? I'm not asking for you to change anything of course, but I would be very interested to hear the justification (technical and biological) - perhaps a quick statement could be added to this end?

joshwlambert commented 2 years ago

Hi @Bisaloo , @alexskeels, & @vboussange.

Firstly, thank you @vboussange for the comments. I will use the specific comments to improve the package.

@alexskeels Apologies for not replying sooner. I am finishing my PhD at the moment and will be submitting the next couple of weeks, and so have been extremely busy. I'm afraid I do not think I'll be able to get to the technical issues that prevented you from completing a full review for the next few weeks.

@Bisaloo Given the comments that have already been received and the fact that I will not be able to invest the time required to address the comments over the coming weeks, is it best to remove this from consideration for publication? I am happy to wait but do not want to waste the time of the reviewers and editors.

Bisaloo commented 2 years ago

Thanks a lot @vboussange and @alexskeels for your reviews! :pray:

@joshwlambert, if you're unsure when you'll be able to work again on this project, then it's indeed a good idea to withdraw & resubmit later. It also looks like we're going to have a conflict of interest soon so it may be a good opportunity to find a new editor at the same time.

joshwlambert commented 2 years ago

@Bisaloo Thanks, I completely agree. I was going to mention the conflict of interest in the next week or so. Once again thank you to @vboussange and @alexskeels for your comments and apologies for not being able to respond to them in a timely fashion.

Bisaloo commented 2 years ago

@openjournals/joss-eics could you please withdraw this submission?

danielskatz commented 2 years ago

@editorialbot withdraw

editorialbot commented 2 years ago

Paper withdrawn.