search

openjournals / joss-reviews

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

[REVIEW]: humanleague: a C++ microsynthesis package with R and python interfaces #629

Closed whedon closed 6 years ago

whedon commented 6 years ago

Submitting author: @virgesmith (Andrew Smith) Repository: https://github.com/virgesmith/humanleague Version: 2.0.2 Editor: @leeper Reviewer: @rdrr1990 Archive: 10.5281/zenodo.1238461

Status

status

Status badge code:

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

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) 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

@rdrr1990, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @leeper know.

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 6 years ago

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks. @rdrr1990 it looks like you're currently assigned as the reviewer for this paper :tada:.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

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

@whedon commands
whedon commented 6 years ago 
Attempting PDF compilation. Reticulating splines etc...
whedon commented 6 years ago

--> Check article proof :page_facing_up: <--

leeper commented 6 years ago

@rdrr1990 Just a quick reminder about this review!

rdrr1990 commented 6 years ago

Thanks! (Haven't forgotten just bested by the quarter system again... I can finally get to this week though.)

On Mon, Apr 9, 2018 at 3:03 AM, Thomas J. Leeper notifications@github.com wrote:

@rdrr1990 https://github.com/rdrr1990 Just a quick reminder about this review!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/629#issuecomment-379701070, or mute the thread https://github.com/notifications/unsubscribe-auth/ASWlXLOSiDkVADx6-s8l81zRkiKEf2uWks5tmzILgaJpZM4SsinU .

-- Pete Mohanty, PhD Science, Engineering & Education Fellow Stanford University Department of Statistics https://sites.google.com/site/petemohanty/

leeper commented 6 years ago

@rdrr1990 Just another ping!

rdrr1990 commented 6 years ago

Apologies again for delay! New to JOSS--do I just post things as they come up? On R side, just built package, testthat runs fine but R CMD check --as-cran returns the classic nuisance error:

Found the following (possibly) invalid URLs:
  URL: http://cran.r-project.org/package=humanleague
    From: README.md
    CRAN URL not in canonical form
leeper commented 6 years ago

@rdrr1990 Easiest thing to do is open issues on the source repo: https://github.com/virgesmith/humanleague. If you want to provide a summary response and/or say anything that doesn't make sense as a standalone issue, feel free to add it here.

rdrr1990 commented 6 years ago

Got it, thanks!

rdrr1990 commented 6 years ago

humanleague offers users the ability to quickly microsynthesize populations from marginals and (optionally) seed data. The package installs nicely for either R or Python and the extensive unit tests run without a hitch on both platforms. The software also builds on a companion paper, Population Synthesis with Quasirandom Integer Sampling which explains the motivation behind Quasirandom Integer Sampling (QIWPS where the 'W' denotes without replacement) as opposed to pseudorandom Iterative Proportional Fitting (IPF). Quasirandom numbers fill the sample space evenly and do not exhibit clumping and other little irregularities like pseudorandom numbers, which the authors argue can be preferable for at least certain applications. The big claim is that QIWPS does a better job at maximizing entropy than IPF.

The paper documents considerable runtime decreases for QIWPS vs. IPF (though the authors acknowledge it's is not an apples-to-apples comparison because it is R vs C++). The paper also demonstrates the R syntax, which is very straightforward. The error messages are also perfectly readable if you e.g. attempt to provide marginals that do not sum to one, which is also appreciated by users.

Overall, humanleague makes a very nice contribution to open source statistical software. My one comment (intended as a recommendation, not a criticism) is that the authors consider writing a vignette (or Github gist, etc.) that takes a practical problem to introduce users to the package. Though the paper is clear enough, most practitioners won't be looking for a discussion of the difference between pseudo and quasi random numbers when they attempt to learn how/whether to use the library. The authors mention ABM as a promising application, so that could certainly be useful. Or pick a recent survey and start with the marginals. Since most will be familiar with $\chi^2$ statistic like the one used in the simulation, that could be an easy way to introduce people to the advantages of humanleague.

leeper commented 6 years ago

@rdrr1990 Excellent review! Thanks so much! Are you able to tick all of the boxes on the review checklist above? If not, can you alert me to anything that's missing?

rdrr1990 commented 6 years ago

Just checked them off... In checking the boxes, I realize I forgot to mention that the authors should add distutils-pytest to the readme's Python dependencies. (Along with the stated numpy that's all I needed to build...). Otherwise happy to give my recommendation for publication. Cheers,

virgesmith commented 6 years ago

I've corrected requirements.txt (its numpy that's missing, not distutils-pytest), I've also corrected the documentation that a user recently pointed out was misleading (i.e. wrong), I've bumped the version and will create a new release unless theres anything else

rdrr1990 commented 6 years ago

Oh, missed that .txt. I just meant in https://github.com/virgesmith/humanleague/blob/master/README.md "Requires Python 3, with numpy, pytest, and distutils-pytest installed" would be clearer (ofc not a big deal). Good to go as far as I'm concerned, great work on the package!

leeper commented 6 years ago

@rdrr1990 Excellent. Thank you, again!

@virgesmith Please update your Zenodo archive and post the new DOI here. We're nearly there!

virgesmith commented 6 years ago

2.0.3 created DOI: 10.5281/zenodo.1238461 Thanks!

leeper commented 6 years ago

@whedon set 10.5281/zenodo.1238461 as archive

whedon commented 6 years ago

OK. 10.5281/zenodo.1238461 is the archive.

leeper commented 6 years ago

Okay, everything looks good. Congrats, @virgesmith, your paper is now accepted at JOSS! (Sorry for the longer-than-expected review process.)

@rdrr1990 Thank you again for your willingness to review for us!

@arfon Over to you.

arfon commented 6 years ago

@rdrr1990 - many thanks for your review here and to @leeper for editing this submission ✨

@virgesmith - your paper is now accepted into JOSS and your DOI is https://doi.org/10.21105/joss.00629 :zap: :rocket: :boom:

whedon commented 6 years ago

:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:

If you would like to include a link to your paper from your README use the following code snippet:

[![DOI](http://joss.theoj.org/papers/10.21105/joss.00629/status.svg)](https://doi.org/10.21105/joss.00629)

This is how it will look in your documentation:

DOI

We need your help!

Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following: