Joss-reviews: [REVIEW]: PolaronMobility.jl: Implementation of the Feynman variational polaron model

Created on 1 Feb 2018  路  33Comments  路  Source: openjournals/joss-reviews

Submitting author: @jarvist (Jarvist Moore Frost)
Repository: https://github.com/jarvist/PolaronMobility.jl
Version: ArXiv-v4
Editor: @lheagy
Reviewer: @yxqd
Archive: 10.5281/zenodo.1336754

Status

status

Status badge code:

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

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

@yxqd, 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 @lheagy know.

### Conflict of interest

Code of Conduct

General checks

  • [x] Repository: Is the source code for this software available at the repository url?
  • [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • [x] Version: Does the release version given match the GitHub release (ArXiv-v4)?
  • [x] Authorship: Has the submitting author (@jarvist) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

  • [x] Installation: Does installation proceed as outlined in the documentation?
  • [x] Functionality: Have the functional claims of the software been confirmed?
  • [x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
  • [x] 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

Software paper

  • [x] Authors: Does the paper.md file include a list of authors with their affiliations?
  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [x] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?
accepted published recommend-accept review

Most helpful comment

@lheagy my review is done. Please also close https://github.com/jarvist/PolaronMobility.jl/issues/1 if you see fit.

All 33 comments

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks. @yxqd 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
Attempting PDF compilation. Reticulating splines etc...
https://github.com/openjournals/joss-papers/blob/joss.00566/joss.00566/10.21105.joss.00566.pdf

@yxqd, thanks for being willing to do the review! Please let me know if you have any questions. I will also jump in and take a look

Hi @jarvist, I have added an issue regarding documentation based on the guidelines above (jarvist/PolaronMobility.jl#1) Please have a look and let me know if there is anything I can clarify. @yxqd may have some comments to add here as well.

Thanks @lheagy. I agree with your comments. Here are some additional points:

Hi @jarvist, your software looks very interesting. There are a few things to fix but it is mostly about documentation. As a first time user, after I follow your installation instruction on your README, it is unclear to me what is the next step, and how to learn to use your software. It would be really helpful to have a detailed example (tutorial), including brief explanation of how the parameters (系-static and 系-optic, for example) were obtained. Also please read the first post in this issue, where a checklist of requirements is presented. For example, "community guidelines" is missing from your software documentation.

It is great that you are using travis-CI to automatically test the software! And it would be nice to increase the coverage to a healthier state.

馃憢 @jarvist: just checking in to see how things are going

馃憢 Hi @jarvist: how are things going with addressing @yxqd's comments?

Hello, sorry things have dragged on a bit - I was doing a fair bit of travelling & moved institution.

I think I've covered most of the issues highlighted with the documentation, and have written an extended 'Examples' component which I hope will help teach the physics. I also believe I now hit the standard JOSS requirements, including a section in the README on contributing.

I had a look at code coverage, but it's already at 100% of the code actually used to generate results! That which is not covered is a work in progress, which might be considerably refactored.

Best.

Thanks for the update @jarvist. @yxqd: would you be willing to take a look at the changes @jarvist has made?

Will do that this weekend. Thanks!

I think the documentation is much improved now. There are some small problems and comments were added to jarvist/PolaronMobility.jl#1 and jarvist/PolaronMobility.jl#2

馃憢 Hi @jarvist: just wanted to check in and see how things are going addressing jarvist/PolaronMobility.jl#1 and jarvist/PolaronMobility.jl#2

https://github.com/jarvist/PolaronMobility.jl/issues/2 is hopefully dealt with.
https://github.com/jarvist/PolaronMobility.jl/issues/1 requires a bit more work still - just need to make the time to finish a gentle refactor + ensure the documentation is current. Thanks for the reminder!

Thanks @jarvist, please keep us posted.

Hi everybody,
Hopefully https://github.com/jarvist/PolaronMobility.jl/issues/1 should be resolved with the latest round of commits culminating in https://github.com/jarvist/PolaronMobility.jl/commit/e4ac60d35d0b0e67efb748c5789b0245bfa897da.

Longer-term, there's definitely some refactoring + more documentation that would be good to write. However, I think that the package is usable in it's current state, and it should be possible to organically improve on it from this point onwards.

Thanks @jarvist. @yxqd, could you please take a look at the changes when you have a chance?

馃憢 Hi @yxqd, @jarvist went through and worked on addressing jarvist/PolaronMobility.jl#1. When you have a chance, could you please take a look? Thanks!

@lheagy thanks for the reminder. Just left my comments at jarvist/PolaronMobility.jl#1. Please review

馃憢 Hi @jarvist: just checking in. Can you please ping here when you are happy with the status of your progress on jarvist/PolaronMobility.jl#1. It is fine to leave this review issue open for a while as you work on it, I just want to make sure it doesn't get forgotten.

Hi @lheagy - thanks for the reminder! I've updated the codes to work with the latest upstream packages, and integrated wkearn's suggestion for how to get PlotPolaron working on an ad-hoc basis at least.
I think getting the inbuilt plotting working for users was the last sticking point? If you are happy to close https://github.com/jarvist/PolaronMobility.jl/issues/1 with https://github.com/jarvist/PolaronMobility.jl/commit/daa4e5789bbb6a6be102cc57b7f9aa9304b3025d#diff-3fb8eca8bec7587dc56e13745886617cL214 , I am as well!

Thanks @jarvist.
馃憢 @yxqd: Thanks for all of your work on this review. Would you be willing to take another look at jarvist/PolaronMobility.jl#1, and if you see that it has been addressed, feel free to close the issue. Thanks!

I've just committed changes which I hope will enable issue https://github.com/jarvist/PolaronMobility.jl/issues/1 to be closed.

Are there any further blocking corrections that are wished for 'Functionality' or 'Documentation' in the tick-boxes above?

@lheagy my review is done. Please also close https://github.com/jarvist/PolaronMobility.jl/issues/1 if you see fit.

Many thanks @yxqd for taking the time on this review! Your input is much appreciated.

@jarvist: could you please archive the software on zenodo or similar and share the doi here?

Dear Both,
Many thanks!

https://github.com/jarvist/PolaronMobility.jl/releases/tag/JOSS
DOI: 10.5281/zenodo.1336754

@whedon commands

Here are some things you can ask me to do:

# List all of Whedon's capabilities
@whedon commands

# Assign a GitHub user as the sole reviewer of this submission
@whedon assign @username as reviewer

# Add a GitHub user to the reviewers of this submission
@whedon add @username as reviewer

# Remove a GitHub user from the reviewers of this submission
@whedon remove @username as reviewer

# List of editor GitHub usernames
@whedon list editors

# List of reviewers together with programming language preferences and domain expertise
@whedon list reviewers

# Change editorial assignment
@whedon assign @username as editor

# Set the software archive DOI at the top of the issue e.g.
@whedon set 10.0000/zenodo.00000 as archive

# Open the review issue
@whedon start review

馃毀 馃毀 馃毀 Experimental Whedon features 馃毀 馃毀 馃毀

# Compile the paper
@whedon generate pdf

@whedon set 10.5281/zenodo.1336754 as archive

OK. 10.5281/zenodo.1336754 is the archive.

馃憢 Hi @arfon, this submission is ready to be published 馃帀

@yxqd - many thanks for your review here and to @lheagy for editing this submission :sparkles:

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

: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 snippets:

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

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.00566">
  <img src="http://joss.theoj.org/papers/10.21105/joss.00566/status.svg" alt="DOI badge" >
</a>

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:

Was this page helpful?
0 / 5 - 0 ratings