Joss-reviews: [REVIEW]: Multiphonon: Phonon Density of States tools for Inelastic Neutron Scattering Powder Data

[whedon]

Submitting author: @Yxqd (Jiao Lin)
Repository: https://github.com/sns-chops/multiphonon
Version: 0.1.1
Editor: @lheagy
Reviewer: @bjmorgan
Archive: 10.5281/zenodo.1162402

Status

Status badge code:

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

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 questions

@bjmorgan, please carry out your review in this issue by updating the checklist below (please make sure you're logged in to GitHub). The reviewer guidelines are available here: http://joss.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @lheagy know.

Conflict of interest

• [x] As the reviewer I confirm that I have read the JOSS conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

• [x] I confirm that I read and will adhere to the JOSS 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 (0.1.1)?
• [x] Authorship: Has the submitting author (@Yxqd) 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)?

[whedon]

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks for JOSS. @bjmorgan 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 as reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all JOSS reviews 😿

To fix this do the following two things:

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

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

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

@whedon commands

[lheagy]

@bjmorgan, @jochym this is ready for you to review.

An informal guideline is that we would like your review in 2 weeks, but sooner (or later) are also ok.

Please carry out your review in this issue by updating the checklist above (please make sure you're logged in to GitHub). There is only one checklist, so please feel free to converse in this issue on the checklist items.

The reviewer guidelines are available here: http://joss.theoj.org/about#reviewer_guidelines

For detailed comments, please create a new issue in the target repository, particularly if those are acceptance-blockers and link them to this thread (https://help.github.com/articles/autolinked-references-and-urls/)

Please let me know if you have any questions or concerns.

Thanks!!

[jochym]

Pardon my stupid question @lheagy , but where is the PDF of the paper? I can see the repo but I cannot locate proper text of the paper.

[lheagy]

Hi @jochym, the paper is here: https://github.com/sns-chops/multiphonon/tree/master/paper. There is a markdown, which renders nicely on github, and @yxqd also included a pdf in that same folder

[jochym]

Thanks. Did I miss something in guidelines? Is this supposed to be standard location?

[lheagy]

You are welcome! It is supposed to be in a standard location (in a paper folder at the root of the repository), however, we don't have this clearly documented in the reviewer guidelines or at the top of the issue. I have started an issue at openjournals/joss#351 so we can clarify that for reviewers

[lheagy]

@tacaswell has also agreed to help with the review. Thanks!!

[yxqd]

@lheagy @bjmorgan @jochym @tacaswell thank you all

[lheagy]

Hi @bjmorgan, @jochym, @tacaswell. Just checking in, how are things going?

[lheagy]

👋 Hi @bjmorgan, @jochym, @tacaswell, I sent you all an email last week checking in. How are things going? Is there anything I can help with?

In terms of checking the boxes: @bjmorgan, would you be willing to take lead? @jochym and @tacaswell, can you give an overview of your thoughts on the status of each of the review items and if you have concerns or feel there are items that need to be improved prior to publication?

If you each prefer, I can give you individual check-lists rather than all filling in one. Please let me know if this would be more clear.

Thanks!

[lheagy]

Just a quick update, @bjmorgan emailed and is willing to take the lead. @jochym and @tacaswell, please be in touch here with your comments. Thanks!

[lheagy]

👋 @bjmorgan, @jochym, @tacaswell: How are things going? Do you guys have an estimate of a timeline of when you will be able to take a look at this? Please let me know if I can be of assistance!

[bjmorgan]

Review questions:

• https://github.com/sns-chops/multiphonon/issues/76
• https://github.com/sns-chops/multiphonon/issues/77
• https://github.com/sns-chops/multiphonon/issues/78
• https://github.com/sns-chops/multiphonon/issues/79
• https://github.com/sns-chops/multiphonon/issues/80

[bjmorgan]

@jochym @tacaswell I'm a solid state computational theorist and only deal with things like phonon densities of states after they have been extracted from experimental data. Are either of you two in a position to check whether the inputs / outputs for the examples look how you would expect?

[lheagy]

thanks @bjmorgan.

👋 @jochym, @tacaswell: have you had a chance to look at the examples?

[jochym]

@lheagy: Thanks for not giving up on me with this review. Here is my opinion on this submission (based on the checklist above):

Conflict of interest

As the reviewer I confirm that I have read the JOSS conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

I confirm that I read and will adhere to the JOSS code of conduct.

General checks

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

Note: _The last point is difficult to judge. The development was clearly carried out outside of github, and only fairly recent changes are tracked. I have no reason to doubt the declarations of the submitter. But I have also no real way to verify them._

Functionality

Installation: Does installation proceed as outlined in the documentation?

Essentially yes, but I have few problems with the procedure:

• The list of dependencies is _huge_. It may be fully justified but from the docs (just two functions in the API) it seems that the dependency management is too careless. Furthermore it depends on packages from four (!) different conda channels. This install is bound to have problems quite soon due to the version drift and incompatibilities - see below.
• The package is written in the heading into obsolescence python 2.7 (EOL in 2020). The process of adoption of python 3 is speeding up quite a bit recently. Multiple new developments in data science already depend on python 3. Including technologies used by the authors - parts of jupyter are already python 3 only. This cannot be a reason for rejection but I would encourage authors to port to python 3 soon - it is easier than expected.
• Such complicated install forces the package into separate virtual environment and makes it difficult or impossible to integrate the package into larger production environment. E.g. it would be impossible to integrate the package into standard jupyterhub environment without major work and inconveniences. This substantially decreases the utility of the software.
• Non-specified dependency on pyversion makes 5 tests to fail. Furthermore this dependency cannot be installed from conda, it's last release on pypi was in 2015 and does not support python 3. Essentially package cannot be fully tested using described install procedure.

Functionality: Have the functional claims of the software been confirmed?

No.

• I was able to run command line example, albeit not cleanly. I have got a following warning:

python2.7/site-packages/histogram/hdf/Loader.py:129: FutureWarning: elementwise comparison failed; returning scalar instead, but in the future will perform elementwise comparison
  if 'storage' in list(dataGroup): # this uses the 'storage' convention

which does not increase confidence in the code - suggesting that it will soon become obsolete.

• Some (5) tests included in the package fail due to the missing pyversion dependence and there are two warnings. One listed above and the second hard to interpret:

tests/backward/singlephonon_sqe2dos_TestCase.py::TestCase::test1c
  /home/jochym/Projects/joss-review/multiphonon/multiphonon/backward/singlephonon_sqe2dos.py:125: UserWarning: Scaling factor to combine DOSes calculated is not stable: 22.1354562834 (using continuous criteria) vs 17.7865062045 (using area criteria)
  You may want to consider adjusting the parameters such as E range (Emax more specifically)
    "You may want to consider adjusting the parameters such as E range (Emax more specifically)" % (scale1, scale2)

• The example notebook cannot be executed to the end. The Converting sample data to powder I(Q,E)... stage produced stack dump with following import error:

ImportError: libjsoncpp.so.0: cannot open shared object file: No such file or directory

probably due to another missing or changed dependency.
Since the notebook is the only fully worked-out example I cannot say the functionality was verified. In fact it will be difficult to even start to fully use the software - having only short, simple and tersely commented example getdos-Al.py to start with.

Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

There are no particular performance claims. The performance during working tests/examples was fully satisfactory.

Documentation

A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?

Yes

Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.

Mostly - the install is a bit complicated but described in enough detail to be followed and completed on the first try. Some dependencies are mentioned, but see my comments above. Some dependencies are missing - non-essential and apparently also essential (see above).

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

Yes, but only the simplest example run to completion and the more elaborated (notebook) failed to execute (see above).

Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?

No. There is only very limited API documentation for two functions. Judging from the simplest example, even this simplest usage requires calling additional API functions (multiphonon.sqe.interp). The only functional documentation is a tersely commented notebook which I was unable to run (see above).

This level of documentation makes the software not suitable for publication

Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?

Yes. But some of them fail.

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

Yes

Software paper

[Y] Authors: Does the paper.md file include a list of authors with their affiliations?
[Y] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[N] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?
Note: Some references have missing doi (e.g. both Nature papers)

Conclusion

Due to the above mentioned deficiencies, in particular:

• Missing functional documentation
• Non-functional examples
• Other smaller deficiencies mentioned above

I cannot recommend publication of this submission at this state.
However, I believe that the mentioned problems can be resolved, which may warrant reconsideration.

[jochym]

@bjmorgan Unfortunately I am theoretician myself - thus I cannot check the inputs but the outputs seem at least reasonable (i.e. when I could get them out - see my report).

[lheagy]

Thanks for your comments @jochym.

@yxqd: could you please take a look at @jochym's comments above? These items are resolvable, and I have started issues where they can be tracked (@jochym: please add to or comment on these as you see fit).

• sns-chops/multiphonon#82
• sns-chops/multiphonon#83
• sns-chops/multiphonon#84
• sns-chops/multiphonon#85

Once these are addressed, we can continue the review process. Please let me know if there is anything I can clarify

[yxqd]

Thank you, @jochym and @lheagy ! I will take actions on these tickets. Some are relatively quick to answer, but some probably won't be resolved immediately. Your patience is much appreciated!

[lheagy]

@yxqd: I have added the pending label. Please ping when you have addressed the issues and we can pick up from there

[yxqd]

Thanks @lheagy. We worked on all these tickets. I closed the DOI ticket since it is straightforward. We (@Fahima-Islam and myself) also worked on the installation and the API documentation. @jochym could you please take a look? First we would like to make sure the installation works for your system. Please give it a try and let us know if there are any problems. Thanks!

[lheagy]

@jochym, would you mind giving a 👍 or further comments regarding @yxqd's comments above when you have a chance? Thanks!

[jochym]

I'll be able to do it on jan/3. Sorry, on holidays right now...

[lheagy]

Sounds good, thanks for the update @jochym. Enjoy the rest of your holidays!

[jochym]

@lheagy see my comments in the review issues.

[lheagy]

Thanks @jochym for all of your feedback! @yxqd, please keep us posted on the progress of the issues, as it looks like

• sns-chops/multiphonon#82
• sns-chops/multiphonon#83
• sns-chops/multiphonon#84

are still in progress

[yxqd]

Thanks @lheagy. We have addressed them in sns-chops/multiphonon#101 and sns-chops/multiphonon#100. @Fahima-Islam is giving it a final check and then we will ask @jochym for another review.

[yxqd]

@lheagy @jochym Please see updates at sns-chops/multiphonon#82

[jochym]

@lheagy, @yxqd : Following my comments in https://github.com/sns-chops/multiphonon/issues/82 I think that after addressing the last minor issues mentioned there the paper can be accepted to be published in JOSS.

[yxqd]

@jochym Thanks again for the review. @jochym @lheagy I addressed some minor fixes in sns-chops/multiphonon#82 and created a couple issues per @jochym 's suggestions to work on later (beyond this review). Please check. Thanks!

[lheagy]

Wonderful, thanks for all of your work @jochym and @yxqd.

I have one last request, @jochym, could you take a look at the checklist at the top of this issues and if all items have been addressed, check them off? Thank you!

@bjmorgan: are there any other comments you have regarding this submission?

[bjmorgan]

@lheagy No. This all looks good to me. I am happy with the assessment from @jochym.

[jochym]

Unfortunately I am unable to modify the checklist @lheagy

[arfon]

@jochym - can you visit this page: https://github.com/openjournals/joss-reviews/invitations and accept the invite at the top of the page? This should allow you to modify the checklist.

[jochym]

Done @lheagy

[lheagy]

Thanks so much for all of your effort on this review!!

@arfon: this submission is ready to be published

[arfon]

@yxqd - At this point could you make an archive of the reviewed software in Zenodo/figshare/other service and update this thread with the DOI of the archive? I can then move forward with accepting the submission.

[yxqd]

Thank you all! Here is the DOI:

[arfon]

@whedon set 10.5281/zenodo.1162402 as archive

[whedon]

OK. 10.5281/zenodo.1162402 is the archive.

[arfon]

@bjmorgan - many thanks for your review here and to @lheagy for editing this submission ✨

@yxqd - your paper is now accepted into JOSS and your DOI is https://doi.org/10.21105/joss.00440 ⚡️ 🚀 💥

[whedon]

: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.00440/status.svg)](https://doi.org/10.21105/joss.00440)

This is how it will look in your documentation:

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 volunteering to review for us sometime in the future. You can add your name to the reviewer list here: http://joss.theoj.org/reviewer-signup.html

[yxqd]

Thank you @arfon