openjournals / joss-reviews

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

[REVIEW]: spatPomp: An R package for spatiotemporal partially observed Markov process models #7008

Open editorialbot opened 4 months ago

editorialbot commented 4 months ago

Submitting author: !--author-handle-->@ionides<!--end-author-handle-- (Edward Ionides) Repository: https://github.com/kidusasfaw/spatPomp Branch with paper.md (empty if default branch): joss Version: 1.0.0 Editor: !--editor-->@gkthiruvathukal<!--end-editor-- Reviewers: @bbolker, @johnlees Archive: 10.5281/zenodo.14058236

Status

status

Status badge code:

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

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

@bbolker & @johnlees, 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 @gkthiruvathukal 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 @johnlees

📝 Checklist for @bbolker

editorialbot commented 4 months 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 4 months ago

Software report:

github.com/AlDanial/cloc v 1.90  T=0.05 s (2129.2 files/s, 286234.7 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
R                               71           1002           2227           6840
C                               12            272             99           1041
Markdown                         5            188              0            746
make                             3             66              0            189
YAML                             4             27              6            137
TeX                              1             16              0            119
C/C++ Header                     1              8             13             44
-------------------------------------------------------------------------------
SUM:                            97           1579           2345           9116
-------------------------------------------------------------------------------

Commit count by author:

   283  Kidus Asfaw
    94  Ed Ionides
    30  Edward Ionides
    25  Aaron A. King
    11  joonhap
     7  allistho
     2  Jesse Wheeler PC
     1  Aaron A King
     1  Edward L Ionides
     1  Haogao Gu
     1  Jifan Li
     1  Kidus
     1  Ning Ning
editorialbot commented 4 months ago

Paper file info:

📄 Wordcount for paper.md is 1208

✅ The paper includes a Statement of need section

editorialbot commented 4 months ago

License info:

🟡 License found: GNU General Public License v3.0 (Check here for OSI approval)

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

OK DOIs

- 10.1080/01621459.2021.1974867 is OK
- 10.18637/jss.v069.i12 is OK

MISSING DOIs

- No DOI given, and none found for title: A tutorial on spatiotemporal partially observed Ma...
- No DOI given, and none found for title: An introduction to sequential Monte Carlo
- No DOI given, and none found for title: Data assimilation fundamentals: A unified formulat...
- No DOI given, and none found for title: Source code for “Bagged filters for partially obse...
- 10.1109/9780470544334.ch9 may be a valid DOI for title: A new approach to linear filtering and prediction ...
- No DOI given, and none found for title: Inference on spatiotemporal dynamics for networks ...
- No DOI given, and none found for title: Code for “Inference on spatiotemporal dynamics for...
- No DOI given, and none found for title: Iterated block particle filter for high-dimensiona...
- No DOI given, and none found for title: Using an iterated block particle filter via spatPo...
- 10.1007/s11222-020-09957-3 may be a valid DOI for title: Inference on high-dimensional implicit dynamic mod...
- 10.1371/journal.pcbi.1012032 may be a valid DOI for title: Informing policy via dynamic models: Cholera in Ha...
- No DOI given, and none found for title: Source code for “Informing policy via dynamic mode...
- 10.2139/ssrn.4082918 may be a valid DOI for title: Mechanisms for the circulation of influenza A (H3N...

INVALID DOIs

- None
editorialbot commented 4 months ago

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

johnlees commented 4 months ago

Review checklist for @johnlees

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

johnlees commented 4 months ago

This is a useful piece of software that is a substantial contribution to the field. I have the following comments on the checklist:

Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).

See https://github.com/kidusasfaw/spatPomp/issues/26

Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

See https://github.com/kidusasfaw/spatPomp/issues/27

Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?

The summary is currently very heavy on technical, statistical language. Leading with the example paragraph and giving more details on a use case would perhaps help. 'metapopulations' and 'unit structure' should be defined to non-experts. Changing the order of sentences in the first paragraph would also be sensible, it feels like the order is almost backwards at the moment.

State of the field: Do the authors describe how this software compares to other commonly-used packages?

This is partially done but only with pomp. It would be useful to a general audience why other packages in this domain (e.g. libBi and odin.dust) and more general statistical packages (e.g. stan) don't fulfill the needs that spatPomp does. The sentence 'We are not aware of alternative packages providing statistically efficient, plug-and-play inference for the general class of SpatPOMP models.' would be improved by citing some existing packages and explaining what they do/don't do instead.

johnlees commented 3 months ago

Issues https://github.com/kidusasfaw/spatPomp/issues/26 and https://github.com/kidusasfaw/spatPomp/issues/27 addressed and I have marked those boxes as complete accordingly

ionides commented 3 months ago

@johnlees, thanks again for the feedback on the spatPomp code and manuscript. I'm unsure of the protocols for the open review process at JOSS, but it seems to make sense to respond now to your two points about the manuscript. The revised version is available as paper.pdf on the joss branch of the spatPomp repo (https://github.com/kidusasfaw/spatPomp/blob/joss/paper.pdf).

  1. The summary is currently very heavy on technical, statistical language. Leading with the example paragraph and giving more details on a use case would perhaps help. 'metapopulations' and 'unit structure' should be defined to non-experts. Changing the order of sentences in the first paragraph would also be sensible, it feels like the order is almost backwards at the moment.

We have rewritten the summary using this feedback. It now starts with introducing a concrete application and moves toward the general situation, whereas the original submission took the reverse order.

  1. It would be useful to a general audience why other packages in this domain (e.g. libBi and odin.dust) and more general statistical packages (e.g. stan) don't fulfill the needs that spatPomp does. The sentence 'We are not aware of alternative packages providing statistically efficient, plug-and-play inference for the general class of SpatPOMP models.' would be improved by citing some existing packages and explaining what they do/don't do instead.

Thank you for pointing out this oversight. Additional discussion of alternative packages is in the extended package tutorial but we agree that this topic should be included also in the relatively brief JOSS overview. We have added some more sentences to the paragraph on the relationship to previous packages, which now reads as follows:

The spatPomp package builds on pomp (King et al. 2016) which is a successful software package for low-dimensional POMP models. Other packages with similar capabilities to pomp include nimble (Michaud et al. 2021), LiBBi (Murray et al. 2015) and mcstate with odin and dust (FitzJohn et al. 2020). All these packages enable plug-and-play inference based on sequential Monte~Carlo. Markov chain Monte Carlo packages, such as stan, have been found to be effective for inference on some POMP models (Li et al. 2018) but they lack the plug-and-play property. Perhaps for that reason, sequential Monte Carlo methods have found broader applicability for this model class. We are not aware of alternative packages to spatPomp that provide statistically efficient, plug-and-play inference for the general class of SpatPOMP models.

johnlees commented 3 months ago

@ionides Thanks for these changes, I think both sections read very well. Thanks also for the small tweaks to the package you already addressed in the issues.

Congratulations and thank you for another useful addition to software packages in this important area. I have marked all the other boxes in my review checklist as complete.

bbolker commented 1 month ago

Review checklist for @bbolker

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

gkthiruvathukal commented 1 month ago

@johnlees Thanks for your detailed review and follow-up. We will await feedback from @bbolker before proceeding with the next steps!

gkthiruvathukal commented 1 month ago

@bbolker Can you provide a status update on your review?

bbolker commented 1 month ago

Thank you for your extreme patience. The package looks fine to me, the vignette is extremely clear and detailed. A few general comments:

details

These are mostly comments on the vignette.

The setup of the package and GitHub repository make it a little bit harder than I'd like to access the underlying materials. For example, the vignettes are understandably set up as shells to import a pre-compiled PDF. That makes sense because the vignette computations would certainly be too slow for testing on CRAN, but where are the vignette sources? They could be included in inst/ somewhere (and a Makefile could be included for building the vignettes from scratch, or at least from stored rds/rda files containing the results of computationally expensive fits ...) I list a bunch of small typos in the vignette below - if the vignette source had been on the repo I could have fixed them myself and submitted a pull request ...

code details

tutorial vignette details

ibpf vignette

other points

bbolker commented 1 month ago

I have completed the checklist and given some suggestions (none of which are necessary to address before considering package acceptable). Is there anything else I need to do to move this (very belated because of me) process along to the next stage?

gkthiruvathukal commented 1 month ago

@bbolker Thanks for your detailed feedback. This is wonderful!

@ionides Can you work to address as many of these as possible and follow up when done? As @bbolker mentioned, these are not blockers for acceptance. Still, we'd like to make sure your submission has maximal impact, which in part will be achieved by making sure your submission is as free of issues as possible.

I'd like to move this submission forward as soon as possible, so please follow up and I can direct you to the next steps.

ionides commented 1 month ago

Thank you, @bbolker and @gkthiruvathukal. It will probably take a couple of weeks to incorporate Ben's thoughtful suggestions, and at that point I'll get back to you.

gkthiruvathukal commented 1 month ago

Thanks, @ionides! I'll follow up then.

ionides commented 1 month ago

Hi @bbolker, Thanks for your detailed feedback. Here is a point-by-point response, with your text in bold. Changes to the JOSS manuscript, paper.pdf, and the spatPomp package are pushed to the github repository. Package changes will not appear on CRAN until the next upload.

1. Thank you for your extreme patience. The package looks fine to me, the vignette is extremely clear and detailed.

General comments

2. It's moderately hard to do non-trivial checks on packages where most of the computation is non-trivial. The package does have some checks. It might be useful in the long run to look at porting tests to tinytest, which can more easily do granular testing.

3. Glad there's a prior/regularization possibility (and a link to the pomp documentation)

4. What if initial values are estimated from dynamical parameters (as in "model starts from equilibrium density of an SIR model") ? (theta_IVP vs theta_RP) ?

_For a stationary model, the initial state may be set at the stationary distribution by specifying $t_0$ sufficiently remote from $t1$ to allow the system to equilibrate. Stocks et al. (2020) provides an example of this using pomp. When following this approach, the model is insensitive to the choice of $f{X_0}(x0| \theta)$.

The RPs are also provided to rinit so they can participate in the initialization.

5. It would be nice if there were syntax tricks to make snippets indentable/highlightable, but this is really a more general pomp question

Details

These are mostly comments on the vignette.

6. Is there a reason that simulate(..., format = "data.frame") gives unit name unitname; as.data.frame(simulate(...)) gives unit name unit?

7. Section 2.8, avoid smart quotes in format = "data.frame"

8. Are extended filters (GIRF, APF etc.) useful/available for non-spatial problems too?

9. "platform for developing new statistical methods" - by authors, or anyone? If the intention is to make this a general platform (analogous to NIMBLE), it might be useful to have developer guidelines.

Section 2 explains the elementary methods used to access properties of the spatPomp model class. These elementary methods are the building blocks available to developers for implementing complex algorithms acting on spatPomp models.

10. I wondered about the possibility of including synthetic likelihood (see Fasiolo and Wood synlik package) as an alternative method (most similar to ABC, although dependent on being able to model the multivariate distribution of summary statistics as a multivariate Gaussian or some extension thereof (e.g. multivariate skew-t)

11. The setup of the package and GitHub repository make it a little bit harder than I'd like to access the underlying materials. For example, the vignettes are understandably set up as shells to import a pre-compiled PDF. That makes sense because the vignette computations would certainly be too slow for testing on CRAN, but where are the vignette sources? They could be included in inst/ somewhere (and a Makefile could be included for building the vignettes from scratch, or at least from stored rds/rda files containing the results of computationally expensive fits ...) I list a bunch of small typos in the vignette below - if the vignette source had been on the repo I could have fixed them myself and submitted a pull request$\dots$

Code details

12. It might be useful to have a choice of tracing frequency/verbosity for the verbose option

__13. Might make sense to rename the 'verbose' option to something package-specific (e.g. options("spatPomp_verbose")), to avoid potential classes with other packages using the same kind of mechanism__

Tutorial vignette details

__14. In general I prefer to see log-likelihoods stated on some scale relative to the best (reference) model (i.e. rel_llik = max(likvec) - likvec, or the opposite for negative log-likelihoods; in the case of Figure 4, that would give values relative to KF (e.g. p. 24)__

15. possibly consider new-style code formatting in the vignette (no prompts or continuation markers) to make cutting and pasting easier

16. Figure 5: show loglik on a log scale from final/max value? (i.e., the large increase in log-likelihood from its initial value makes it harder to distinguish changes during the plateau - showing the log of relative log-likelihood may seem a little redundant but makes small changes easier to see ...)

17. p. 25 "contains a parameter estimate [and] its log-likelihood"

Done.

18. p. 27 "are counting process[es]"

Done.

19. p. 29 "We [construct] the model"

Done.

20. p. 30 could single quotes in code be type set as straight quotes ' rather than single back-quotes ´?

Done.

21. p. 32 not clear what the "parameter vector of scientific interest" is ...

A fair point. We meant to make a general assertion that the user will probably put in a parameter vector that seems meaningful to them, but we've deleted "of scientific interest" since it is redundant and evidently does not add clarity.

__22. $R_0 = 30$ seems high for measles (I do know that some studies have estimated it as that high or higher ...)__

Various time series investigations have found a preference for higher $R_0$ values for measles than some other traditional estimation methods, for reasons not yet fully understood.

23. p. 35 italicize Leptospira interrogans?

Done.

ibpf vignette

24. p.7 "provide[s] an adequate"

25. p. 7 delete comma after "but"

26. "Jensen's [inequality]"

27. "This is [in] contrast"

28. p. 9 "for one [parameter]"

29. p. 12 "agree on the most problematic observation[s]" ?

30. p. 14 "For our measles spatPomp [example]"

31. Fig 8: are the trace plots constant for the last parameters since these were held fixed in the example? Is it true that we shouldn't interpret the trace plots in quite the same way as for MCMC (since we are not trying to take a sample, i.e. we don't care about mixing or burn-in)?

32. Wheeler et al; "Ionides, E. L." (not j.)"

Other points

33. plot method could/should add a y-axis label to the plot that indicates the log10(x+1) transformation? (Why not use scale_y_log10() [which automatically puts zero values (log(x) → -∞) at the edge of the plot] or the log1p transformation?

34. Titles of some help pages are noninformative (runit_measure, munit_measure)

35. plot page headings could be improved

36. use FALSE, TRUE rather than T, F; clarify that log=TRUE uses log(1+x)

ionides commented 3 weeks ago

Hi @gkthiruvathukal, I realized I did not address you in my response to Ben Bolker. Just to clarify, I have addressed his comments in the package, paper and tutorials. Please let me know if you need more from me at this time. Best, Ed.

gkthiruvathukal commented 3 weeks ago

@ionides Thanks much! I do keep an eye on the issue, so it's all good.

@bbolker Can you let me know whether all comments are addressed to your satisfaction?

bbolker commented 3 weeks ago

Yes, they are. Thanks.

gkthiruvathukal commented 3 weeks ago

Thanks, @bbolker. Given that @johnlees's feedback has also been addressed, I can move toward acceptance. I will generate the checklist of steps that need to be completed by you (@ionides). Stay tuned.

gkthiruvathukal commented 3 weeks ago

@editorialbot commands

editorialbot commented 3 weeks ago

Hello @gkthiruvathukal, here are the things you can ask me to do:


# List all available commands
@editorialbot commands

# Add to this issue's reviewers list
@editorialbot add @username as reviewer

# Remove from this issue's reviewers list
@editorialbot remove @username from reviewers

# Get a list of all editors's GitHub handles
@editorialbot list editors

# Assign a user as the editor of this submission
@editorialbot assign @username as editor

# Remove the editor assigned to this submission
@editorialbot remove editor

# Remind an author, a reviewer or the editor to return to a review after a 
# certain period of time (supported units days and weeks)
@editorialbot remind @reviewer in 2 weeks

# Adds a checklist for the reviewer using this command
@editorialbot generate my checklist

# Set a value for version
@editorialbot set v1.0.0 as version

# Set a value for branch
@editorialbot set joss-paper as branch

# Set a value for repository
@editorialbot set https://github.com/organization/repo as repository

# Set a value for the archive DOI
@editorialbot set 10.5281/zenodo.6861996 as archive

# Mention the EiCs for the correct track
@editorialbot ping track-eic

# Run checks and provide information on the repository and the paper file
@editorialbot check repository

# Check the references of the paper for missing DOIs
@editorialbot check references

# Generates the pdf paper
@editorialbot generate pdf

# Recommends the submission for acceptance
@editorialbot recommend-accept

# Generates a LaTeX preprint file
@editorialbot generate preprint

# Flag submission with questionable scope
@editorialbot query scope

# Get a link to the complete list of reviewers
@editorialbot list reviewers

# Creates a post-review checklist with editor and authors tasks
@editorialbot create post-review checklist

# Open the review issue
@editorialbot start review
gkthiruvathukal commented 3 weeks ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

gkthiruvathukal commented 3 weeks ago

@ionides Please see above Additional Author Tasks!

ionides commented 2 weeks ago

@gkthiruvathukal, this is taking a few days because we have decided it is time to move spatPomp to an organization (github.com/spatPomp-org/spatPomp rather than github.com/kidusasfaw/spatPomp). That should be done anyhow, since the original maintainer, @kidusasfaw, has less time for this project in his current job. The organization framework will be better for future growth, with a core development team that has published procedures (in https://github.com/spatPomp-org/.github/blob/main/profile/Governance.md) for incorporating new developers. This does not directly affect the software, though it does help respond to peer review comments about clarifying the process for developers to join the project.

I understand it adds a logistic issue with JOSS to move the location of the project repo at this time. If you have a strong preference for us to make the move after finishing the peer review process, we could do that. My assumption was that any improvements would be best done before finalizing the JOSS version. So, my working hypothesis is that we will get back to you when the spatPomp repo is fully installed at spatPomp-org and released from there to Zenodo. Please let me know if you recommend any different course of action.

gkthiruvathukal commented 2 weeks ago

@ionides I actually think this is a good idea to move it to an organization. I believe the system is designed to allow this change but don't know whether you or I would need to update the repository associated with the submission. I may have to read the manual. In any event, let me know when ready and I think we can easily figure this out.

ionides commented 2 weeks ago

Hi @gkthiruvathukal, Here are the responses to the author tasks.

As I mentioned, the repository for the project has moved to https://github.com/spatPomp-org/spatPomp The current version of paper.md is on the joss branch of this repository. Please let me know if there's anything else I can do to help.

Best, Ed

Additional Author Tasks After Review is Complete

Done.

Version 1.0.0

https://doi.org/10.5281/zenodo.14058236

Done.

Done.

ionides commented 1 week ago

Hi @gkthiruvathukal,

A couple of updates:

  1. spatPomp 1.0.0 is now on CRAN

  2. I updated the joss and master branches on https://github.com/kidusasfaw/spatPomp to match https://github.com/spatPomp-org/spatPomp so it will not matter which site JOSS uses for the final version of the manuscript. Once this issue is resolved, we will archive https://github.com/kidusasfaw/spatPomp and continue development only on https://github.com/spatPomp-org/spatPomp.

Thanks, Ed

ionides commented 2 days ago

Hi @gkthiruvathukal https://github.com/gkthiruvathukal, I'm not sure of the procedure at this point, so I'm just checking in for confirmation that I have completed everything on the author checklist (with responses in this thread) so we are waiting for JOSS to get back to us with any further requests. Thanks, Ed.

Edward L. Ionides Associate Chair for Undergraduate Studies and Professor, Department of Statistics, University of Michigan 1085 South University, Ann Arbor, MI 48109-1107 email: @.*** phone: 734 615 3332 office: 453 West Hall

On Thu, Nov 7, 2024 at 1:12 PM George K. Thiruvathukal < @.***> wrote:

@ionides https://github.com/ionides I actually think this is a good idea to move it to an organization. I believe the system is designed to allow this change but don't know whether you or I would need to update the repository associated with the submission. I may have to read the manual. In any event, let me know when ready and I think we can easily figure this out.

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/7008#issuecomment-2462917686, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKAFT2QPYA3NLMSBLQQX4TZ7OURJAVCNFSM6AAAAABLDRP5LKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINRSHEYTONRYGY . You are receiving this because you were mentioned.Message ID: @.***>

gkthiruvathukal commented 2 days ago

@ionides You're good. I will work on this and let you know if I need anything else.

gkthiruvathukal commented 2 days ago

@editorialbot set 1.0.0 as version

editorialbot commented 2 days ago

Done! version is now 1.0.0

gkthiruvathukal commented 2 days ago

@editorialbot set 10.5281/zenodo.14058236 as archive

editorialbot commented 2 days ago

Done! archive is now 10.5281/zenodo.14058236

gkthiruvathukal commented 2 days ago

@ionides I set the version but you did not do a Release on GitHub. Your last version is 0.35. Can you please tag and release on GitHub, too? CRAN releases are not the same as a GitHub releases.

ionides commented 2 days ago

@gkthiruvathukal, the 1.0.0 release is tagged on the new website for the organization at https://github.com/spatPomp-org/spatPomp. Perhaps you were being directed to the old website at https://github.com/kidusasfaw/spatPomp, which is indeed at 0.35. That repository is archived and no longer in development. It could be brought back to life and updated, if that is necessary. Alternatively, if this explanation is sufficient for you to keep moving, that would be great. Thanks, Ed.