Joss-reviews: [REVIEW]: bem: Modeling for Neutron Bragg-Edge Imaging

[whedon]

Submitting author: @Yxqd (Jiao Lin)
Repository: https://github.com/ornlneutronimaging/braggedgemodeling
Version: v0.1.1
Editor: @katyhuff
Reviewer: @stuartcampbell, @mikapfl, @katyhuff
Archive: 10.5281/zenodo.1466048

Status

Status badge code:

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

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

@stuartcampbell & @mikapfl & @katyhuff, 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 @katyhuff know.

✨ Please try and complete your review in the next two weeks ✨

Review checklist for @stuartcampbell

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 (v0.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?
• [ ] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

Review checklist for @mikapfl

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 (v0.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)?

Review checklist for @katyhuff

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 (v0.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, a robot that can help you with some common editorial tasks. @stuartcampbell, 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:

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

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

:point_right: Check article proof :page_facing_up: :point_left:

[katyhuff]

First pass review (attention @yxqd ):

Functionality

• Installation: Installation isn't complete, tests fail due to lack of documentation about diffpy dependency. See this issue: https://github.com/ornlneutronimaging/braggedgemodeling/issues/23
• Other functionality claims rely on proper installation instructions. I'll continue this review when https://github.com/ornlneutronimaging/braggedgemodeling/issues/23 is closed.

Paper

• Authorship: It seems @JeanBilheux has contributed just as many commits as second author Song. Should @JeanBilheux be included in the author list? Without this author, the author list seems incomplete.
• Software Paper: There are various places where "neutron" is arbitrarily capitalized. It is not a proper noun.

[yxqd]

Thanks @katyhuff. Song made significant contribution to the software by providing the first implementation of the algorithm in Excel :) ; performing the research using the code; and providing numerous feedback.

I will fix the installation instructions.

[yxqd]

@mikapfl @katyhuff installation instruction revised. Please check ornlneutronimaging/braggedgemodeling#23

[katyhuff]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

:point_right: Check article proof :page_facing_up: :point_left:

[katyhuff]

@yxqd This tool is small, certainly, but a functional utility. My only remaining concern is with the examples question.

Part of the review for JOSS is with regards to the understandability and usability of the code. Toward this aim, we ask the question: _Do the authors include examples of how to use the software (ideally to solve real-world analysis problems)._

Indeed, as an exploring user, I immediately found and ran your tutorial.ipynb file. I even thought I understood what's going on it. However, it was pretty brief and I certainly wouldn't call it a tutorial or a "real world" example since the problems being approached aren't described at all in the notebook. Later, I saw that the corresponding page in the docs is the exact same example, but with just a few extra sentences to help the user: https://ornlneutronimaging.github.io/braggedgemodeling/tutorial.html . Even though this page is still very minimal, it is far easier to understand!

Accordingly, I'd like to suggest perhaps fleshing out, just a bit, these examples so that it's clearer who should use this and what analysis really calls for it. It would generally be ideal if the notebook in the source code included the same level of explanation as the readthedocs site.

Otherwise, this looks ok to me and I was able to run the tests and tutorial notebook successfully.

[stuartcampbell]

Hi @yxqd,
I installed using conda, worked without an issue. Although as a small usability thing, I would change the install to be a single line: $ conda install braggedgemodeling -c condo-forge - this means that you are not permanently. changing the default channels.

Running through the tutorials the answers look like something I would expect. As a small suggestion, I would put images of the expected example plots in the tutorial so users can check that they are seeing what is expected. I know the images are in the notebook, but as you point people at the website then I would put them there as well. Also, maybe put a link to the notebook on the tutorial webpage to make it easier for people to find.

I did experience some RuntimeWarnings, I have created an issue for them (see https://github.com/ornlneutronimaging/braggedgemodeling/issues/24) but I don't think that they have to be fixed for this review.

I think overall the package looks good and the only outstanding issue I can see is that your references do not include the DOIs for them. I've opened the issue https://github.com/ornlneutronimaging/braggedgemodeling/issues/25

After that is done I think all the requirements are satisfied.

[katyhuff]

Good catch @stuartcampbell ... I completely failed to notice the missing DOIs!

[yxqd]

Thank you @katyhuff @stuartcampbell for the comments. I will flesh out the notebook and the documentation as you suggested.

[yxqd]

@katyhuff @stuartcampbell documentation was updated as you suggested. Please check https://ornlneutronimaging.github.io/braggedgemodeling/tutorial.html and https://github.com/ornlneutronimaging/braggedgemodeling/blob/master/notebooks/tutorial.ipynb.

DOIs were added for journal papers referenced. I cannot find the DOI for the phd thesis referenced, however. Please check https://github.com/ornlneutronimaging/braggedgemodeling/blob/master/JOSS-paper/paper.bib

Thanks!

[mikapfl]

Hi,

unfortunately (for the review) I'm in vacations now and will only be able to continue with the review in two weeks.

Cheers,

Mika

Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet.

[stuartcampbell]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

:point_right: Check article proof :page_facing_up: :point_left:

[stuartcampbell]

@yxqd the bib file looks good to me now. @katyhuff does @yxqd need to either update the release version to make sure the correct .bib file and paper are in the released git tag ?

[katyhuff]

@stuartcampbell : An updated doi will be created in the final step before acceptance, if a new release is desired, to reflect review-related changes to the paper and code, many authors do issue a new (minor or patch) release version at that stage as well.

@yxqd should wait until the acceptance to do that though, so that this task only needs to be done once. I would like to wait for @mikapfl's review before that stage. Though I understand it may be a couple more weeks from now, I hope that won't put an undue burden on your development process.

[yxqd]

@mikapfl thanks for the note.

@stuartcampbell thanks. I just closed ornlneutronimaging/braggedgemodeling#25.

@katyhuff thanks. sounds a good plan to me

[mikapfl]

Hi,

I looked at the software and documentation today and found it generally easy to follow and the tutorial to be enough to get me started (I can't judge if this is close to a real-world analysis problem, because I don't know how complicated the real world is in the bragg-edge neutron diffraction camp). However, I found a few usability and documentation issued, which I have reported:

• ornlneutronimaging/braggedgemodeling/issues/28
• ornlneutronimaging/braggedgemodeling/issues/29
• ornlneutronimaging/braggedgemodeling/issues/30

None of them are show stoppers, but I think that new users could benefit more from the documentation and the API would be easier to use if they would be fixed.

I'll check the remaining items on the review to-do list tomorrow.

Cheers

Mika

[mikapfl]

Hi,

regarding the review question if all functional claims are implemented in the software, I'm having problems to understand if you actually implement

Calculate diffraction peaks data (d-spacing, form factors, etc.) according to sample crystal structure

At least, this seems to be missing from the documentation (tutorial or API documentation) - how would I, say, calculate a form factor from a sample crystal structure? I haven't opened an issue in the bem issue tracker for this since I'm feeling like I'm missing something here, maybe you can comment, @yxqd ?

Cheers

Mika

[mikapfl]

Hi,

regarding the citations: The citations are linked appropriately and as far as I could see are also relevant. Only one thing for the citations to be done: I think the right citation for Sven Vogel's dissertation would be
Vogel, S. (2000). A rietveld-approach for the analysis of neutron time-of-flight transmission data (PhD thesis). Retrieved from https://macau.uni-kiel.de/receive/dissertation_diss_00000330.
At the moment it reads "Verlag nicht ermittelbar" which is just "publisher unknown" in German.

Cheers

Mika

[yxqd]

Thanks, @mikapfl, for the comments. I will check on the issues.

Regarding "Calculate diffraction peaks data (d-spacing, form factors, etc.) according to sample crystal structure": this is done in the bem.diffraction module. d-spacing and form factor are essential to the Bragg-edge spectrum; without them the Bragg edge curves will not have any edges: each edge is located at a d-spacing position. The "jump" of intensities around the edge is mostly influenced by the form factor. One convenience provided by this code is that it can calculate the d-spacings and form factors from the atomic structure without any effort on the user. However, the intended usage of this package is to calculate the Bragg edge curve, not too much about the d-spacings and form factors only by themselves.

Thanks for finding the problem in citation, I will fix it.

[yxqd]

@mikapfl Please review ornlneutronimaging/braggedgemodeling#23, ornlneutronimaging/braggedgemodeling#28, ornlneutronimaging/braggedgemodeling#29, ornlneutronimaging/braggedgemodeling#30. Thanks.

[mikapfl]

@yxqd bem.diffraction seems to be entirely undocumented. I understand that you need it mainly "behind the scenes" to calculate the full Bragg edge curve, so maybe you should not put "Calculate diffraction peaks data (d-spacing, form factors, etc.) according to sample crystal structure" as the first (and thus implied to be most important) feature in the description of features in the README. If users of this package are mainly interested in Bragg edge curves, put those front and center in your description of features. You can, if you want to, still mention that you of course also calculate diffraction peak data to be able to calculate a Bragg edge curve, but I think you should somehow make clear that it is rather an internal thing.

Cheers

Mika

[katyhuff]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

:point_right: Check article proof :page_facing_up: :point_left:

[katyhuff]

I believe the concern regarding references has been handled. This leaves just the final check boxes by @mikapfl regarding functionality and documentation of the API. Once those are handled, we should be good to go! @yxqd I hope you'll let us know when you believe these have been handled.

[yxqd]

@mikapfl @katyhuff I reworded the "features" to hide the details about diffraction calculation. Please check.

[mikapfl]

Hi,

thanks, that solves the remaining issue for me, from my side, everything has been handled now. (-:

Cheers

Mika

[yxqd]

@mikapfl thanks. @katyhuff I believe all items have been handled.

[katyhuff]

Excellent! Thank you @mikapfl and @stuartcampbell for the reviews! I know it's a lot of work, and I think you both conducted very helpful and thorough reviews.

Thank you @yxqd for this interesting submission, and for engaging actively in the review process.
Could you please 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! (You may want to be sure to double check the paper, all minor details, etc. before creating the archive).

[katyhuff]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

:point_right: Check article proof :page_facing_up: :point_left:

[stuartcampbell]

@katyhuff You are welcome.

[yxqd]

Thank you @katyhuff @stuartcampbell @mikapfl. Here is the doi:

[katyhuff]

@whedon set 10.5281/zenodo.1466048 as archive

[whedon]

OK. 10.5281/zenodo.1466048 is the archive.

[katyhuff]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

:point_right: Check article proof :page_facing_up: :point_left:

[katyhuff]

Thanks again to @stuartcampbell and @mikapfl for your helpful reviews.
Congratulations @yxqd , your paper is ready to be accepted!

@arfon We're ready to accept this submission, so it's over to you!

[arfon]

@whedon accept

[whedon]

Attempting dry run of processing paper acceptance...

[whedon]

Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/17

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/17, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.
@whedon accept deposit=true

[arfon]

@whedon accept deposit=true

[whedon]

Doing it live! Attempting automated processing of paper acceptance...

[whedon]

🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨

Here's what you must now do:

1. Check final PDF and Crossref metadata that was deposited :point_right: https://github.com/openjournals/joss-papers/pull/18
2. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.00973
3. If everything looks good, then close this review issue.

4. Party like you just published a paper! 🎉🌈🦄💃👻🤘

   Any issues? notify your editorial technical team...

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

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

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

reStructuredText:
.. image:: http://joss.theoj.org/papers/10.21105/joss.00973/status.svg
   :target: https://doi.org/10.21105/joss.00973

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 doing either one (or both) of the the following:

• 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
• Making a small donation to support our running costs here: https://www.flipcause.com/secure/cause_pdetails/Mjk3Nzk=

[yxqd]

Thank you @katyhuff @arfon

[mikapfl]

Hi,

the "cite as" part in http://joss.theoj.org/papers/10.21105/joss.00973 looks weird, is that intended?

Cheers

Mika

[arfon]

Nope. That's a bug. I'll fix early next week.

On Fri, Oct 19, 2018 at 07:58, Mika Pflüger [e-mail]
wrote:

Hi,

the "cite as" part in http://joss.theoj.org/papers/10.21105/joss.00973
looks weird, is that intended?

Cheers

Mika

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/openjournals/joss-reviews/issues/973#issuecomment-431339111,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAARg1SYbzgqXVjtCY9hgTS5KNNp-txkks5umb5TgaJpZM4W1yL4
.

[arfon]

OK that should be fixed now.