[REVIEW]: Gillespie.jl: Stochastic Simulation Algorithm in Julia about joss-reviews HOT 21 CLOSED

@sdwfrost I would recommend to register your package to simplify installation and to tag your package.

@whedon I would offer to review this. How would I proceed?

from joss-reviews.

@whedon I would offer to review this. How would I proceed?

@vchuravy - great! I've just added you to the JOSS reviewers team. Please do the following before starting this review:

• Accept the invite to join the 'joss-reviewers' team on GitHub that you should have received.
• Familiarize yourself with the JOSS reviewer guidelines
• Complete this request for information about your background: openjournals/joss#138

Any questions, please don't hesitate to get in touch.

from joss-reviews.

Ok, done.

from joss-reviews.

@sdwfrost I will add my thoughts here:

• Code-coverage seems not to be working
• Performance: Could you add some performance numbers comparing the C and R version with the Julia version.
• Tests: The tests you are using are testing functionality. Would it be possible to add correctness tests?
• Documentation: It is great to see that you have inline docstrings. Maybe you could use them to generate a documentation (with Documenter.jl)
• Please add DOIs to the .bib file

from joss-reviews.

This software consists of 100+ lines of Julia code and is a reimplementation of existing code. I would like to ask the author to confirm here that he thinks this software is a valuable contribution to research. I want to point out that once published this work will be visible for a long time. Adding an example of having used this tool in research would be a good idea.

Also a speed claim should show numbers and a version of 0.01 looks alpha to me.

from joss-reviews.

@vchuravy Thanks for the feedback!

• The package is on its way to being in METADATA.jl (overlooked version number for Julia in requires)
• Coverage is now fixed (silly error) - at 93%.
• Happy to put in the comparisons. Would R and Rcpp suffice?
• As these are stochastic simulations, it's a bit hard to check. I can test for consistency (such that examples give fixed outputs).
• I haven't used Documenter.jl. Will give it a go.
• Will add DOIs.

@pjotrp Thanks for the feedback too!

• There aren't many tools out there that allow one to easily run these types of simulations - at least, those that aren't heavily based on SBML. The paper for GillespieSSA only has 28 citations on Google Scholar - not great for a tool - but other packages e.g. hybridModels use it, and other packages (e.g. POMP) have their own custom implementation. The overall style of the package (and Julia-specific ways of speeding things up), was originally borrowed by @rveltz, and we're now working on implementations of more research-level algorithms. So, despite being a small library (it'd be longer in Java ;) ), on balance, I say that it's worth it.
• The version number is following Julia guidelines (as Julia is < 1.0 right now), rather than necessarily reflecting an alpha stage.

I also see the 'Community' checkbox is unchecked. I'll add a section there too.

Thanks again @vchuravy @pjotrp @arfon @whedon I think that this is a great initiative.

from joss-reviews.

Dear Simon,

I don’t understand the meaning of this mail. Is it some reviews from joss for Gillespie.jl?

Romain

On 21 Jul 2016, at 12:27, Simon Frost [email protected] wrote:

@vchuravy https://github.com/vchuravy Thanks for the feedback!

The package is on its way to being in METADATA.jl (overlooked version number for Julia in requires)
Coverage is now fixed (silly error) - at 93% https://coveralls.io/github/sdwfrost/Gillespie.jl?branch=master.
Happy to put in the comparisons. Would R and Rcpp suffice?
As these are stochastic simulations, it's a bit hard to check. I can test for consistency (such that examples give fixed outputs).
I haven't used Documenter.jl. Will give it a go.
Will add DOIs.
@pjotrp https://github.com/pjotrp Thanks for the feedback too!

There aren't many tools out there that allow one to easily run these types of simulations - at least, those that aren't heavily based on SBML. The paper for GillespieSSA only has 28 citations on Google Scholar - not great for a tool - but other packages e.g. hybridModels use it, and other packages (e.g. POMP) have their own custom implementation. The overall style of the package (and Julia-specific ways of speeding things up), was originally borrowed by @rveltz https://github.com/rveltz, and we're now working on implementations of more research-level algorithms. So, despite being a small library (it'd be longer in Java ;) ), on balance, I say that it's worth it.
The version number is following Julia guidelines (as Julia is < 1.0 right now), rather than necessarily reflecting an alpha stage.
I also see the 'Community' checkbox is unchecked. I'll add a section there too.

Thanks again @vchuravy https://github.com/vchuravy @pjotrp https://github.com/pjotrp @arfon https://github.com/arfon @whedon https://github.com/whedon I think that this is a great initiative.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #42 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/ABhKBCdjDAN2EOiFA3-3-LPlszJ5rThkks5qX0mFgaJpZM4JRJlz.

from joss-reviews.

Hi @rveltz - I just mentioned you in conversation in my responses to referees (as it says on the bottom of the email) - this also gives you the option to mute the thread.

from joss-reviews.

Hi,

I did not know you could share reviews like that,

Romain

On 21 Jul 2016, at 13:34, Simon Frost [email protected] wrote:

Hi @rveltz https://github.com/rveltz - I just mentioned you in conversation in my responses to referees (as it says on the bottom of the email) - this also gives you the option to mute the thread.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #42 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/ABhKBH4VRpSPbXeAXPX1PsDUqrpgbs1Fks5qX1lFgaJpZM4JRJlz.

from joss-reviews.

Thanks for review @vchuravy and the updates @sdwfrost. @sdwfrost please let me know when you've had a chance to complete the other items.

from joss-reviews.

@sdwfrost Thank for including the benchmarks. Just for your information @benchmark takes the parameters samples and seconds as limits. So in your case the benchmark is not running for a 1000 samples because the time limit is exceeded, to turn that of you can set seconds=Inf. Also it is preferential to write $x0 instead of x0, otherwise you will also benchmark the cost of accessing a global variable (expensive in Julia).

@benchmark ssa(x0,F,nu,parms,tf) samples=1000
# should be
@benchmark ssa($x0,$F,$nu,$params,$tf) samples=1000 seconds=Inf

Also the times for R,Rccp,and SSA should probably be divided by N so that they are more readily comparable.

from joss-reviews.

@vchuravy I didn't get round to finishing the writeup on the benchmarks; thanks for the tips on @benchmark; I didn't know that you could use interpolation. Working through Documenter.jl right now. Will update here when the rest of the items are complete. Thanks again for your input.

from joss-reviews.

@vchuravy I've added some documentation via Documenter.jl - is this OK? - and updated the README to include proposed enhancements. I'm working on updating the benchmarks.

from joss-reviews.

Dear @vchuravy @arfon @pjotrp

I think I've addressed all your comments now. Please let me know if there is anything I have missed. In summary

• Documentation is written using Documenter.jl
• Updated benchmarks are included for Julia and R/Rcpp. Although the plot doesn't show up in the IJulia notebook, the output is comparable between a handcoded version and Gillespie.jl. These benchmarks are summarised in the README.md. I hope that these results provide sufficient rationale for inclusion of Gillespie.jl in JOSS. I have also included a benchmark file in the repository.
• The tests now include checks that the output is at fixed for a given random number seed (in addition to checking that all the examples run). Code coverage is 95%
• I've added some material to the end of the README with respect to future directions etc.
• dois added to the bib file.
• Release v0.0.1 added to the repository (rationale for numbering is following the Julia convention).
• Various badges have been added, including links to stable and latest docs, Coveralls.io, Waffle.io, and JOSS.

Thanks!

from joss-reviews.

@arfon How would one regenerate the pdf? The .bib now includes DOI's, but they don't show up the the pdf.
@sdwfrost Great work! The codecoverage seems to be off again. One last question from my side. Have you tested compatibility with Julia v0.5 (the rc0 was just announced)?

Otherwise this is a green light from me.

from joss-reviews.

@arfon How would one regenerate the pdf? The .bib now includes DOI's, but they don't show up the the pdf.

@vchuravy here's the updated PDF: 10.21105.joss.00042.pdf

@sdwfrost Great work! The codecoverage seems to be off again. One last question from my side. Have you tested compatibility with Julia v0.5 (the rc0 was just announced)?

@sdwfrost - once you've addressed this comment I think we're good to accept this submission 🎉

from joss-reviews.

@arfon @vchuravy Compatibility now checked against v0.4.6 and v0.5, all tests passing. I get some deprecation warnings on v0.5, but future releases will fix those.

@vchuravy I was getting some funny badges on Chrome for Coveralls (fine on Firefox) - click through, and you'll see coverage is still good.

from joss-reviews.

@sdwfrost Perfect thanks.

from joss-reviews.

@sdwfrost - as a final step, could you make an archive of the code in Zenodo/figshare? Please update this thread with the DOI of the code archive.

from joss-reviews.

@arfon DOI from Zenodo: 10.5281/zenodo.59127

from joss-reviews.

Thanks @sdwfrost. Your paper is now accepted into JOSS and your DOI is http://dx.doi.org/10.21105/joss.00042 🚀 🎉 💥

Thanks for the review @vchuravy

from joss-reviews.