Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REVIEW]: NuclearToolkit.jl: A Julia package for nuclear structure calculations #4694

Closed
editorialbot opened this issue Aug 22, 2022 · 72 comments
Assignees
Labels
accepted Julia published Papers published in JOSS recommend-accept Papers recommended for acceptance in JOSS. review TeX Track: 3 (PE) Physics and Engineering

Comments

@editorialbot
Copy link
Collaborator

editorialbot commented Aug 22, 2022

Submitting author: @SotaYoshida (Sota Yoshida)
Repository: https://github.com/SotaYoshida/NuclearToolkit.jl
Branch with paper.md (empty if default branch): main
Version: 0.2.5
Editor: @rkurchin
Reviewers: @mdavezac, @villaa
Archive: 10.5281/zenodo.7314918

Status

status

Status badge code:

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

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) by leaving comments 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

@mdavezac & @villaa, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review.
First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @rkurchin know.

Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest

Checklists

📝 Checklist for @mdavezac

📝 Checklist for @villaa

@editorialbot
Copy link
Collaborator Author

Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.

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

@editorialbot commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@editorialbot generate pdf

@editorialbot
Copy link
Collaborator Author

Software report:

github.com/AlDanial/cloc v 1.88  T=0.14 s (365.9 files/s, 178132.2 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Julia                           34            810            707          22574
TeX                              1             10              0            218
Markdown                         8             54              0            198
TOML                             5             35              2            133
YAML                             3              0              2             84
-------------------------------------------------------------------------------
SUM:                            51            909            711          23207
-------------------------------------------------------------------------------


gitinspector failed to run statistical information for the repository

@editorialbot
Copy link
Collaborator Author

Wordcount for paper.md is 448

@editorialbot
Copy link
Collaborator Author

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- h10.1016/S0146-6410(01)00159-4 is OK
- 10.1103/RevModPhys.77.427 is OK
- 10.1103/RevModPhys.92.015002 is OK
- 10.1146/annurev-nucl-101917-021120 is OK
- 10.1016/j.physrep.2011.02.001 is OK
- 10.1088/0031-8949/91/8/083007 is OK
- 10.1103/RevModPhys.81.1773 is OK
- 10.3389/fphy.2020.00098 is OK
- 10.1140/epja/i2003-10096-0 is OK
- 10.1093/ptep/ptac057 is OK
- 10.1017/CBO9780511596834 is OK
- 10.1016/j.cpc.2013.07.022 is OK
- 10.1016/j.nds.2014.07.022 is OK
- 10.1016/j.cpc.2019.06.011 is OK

MISSING DOIs

- None

INVALID DOIs

- None

@editorialbot
Copy link
Collaborator Author

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@SotaYoshida
Copy link

@rkurchin Thank you for your kind help and editorial work.
I have a question:
If a reviewer points out a bug or typo in the code, is it acceptable to update the code during the review process and should it be tagged?

@rkurchin
Copy link

@SotaYoshida it is not only acceptable, it is highly encouraged! As for tagging, we will ask you to issue a new release tag at the end of the review process. If you want to do so along the way, that's fine, but it's not necessary.

@SotaYoshida
Copy link

I got it. Thanks!

@mdavezac
Copy link

mdavezac commented Aug 28, 2022

Review checklist for @mdavezac

Conflict of interest

  • I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

General checks

  • Repository: Is the source code for this software available at the https://github.com/SotaYoshida/NuclearToolkit.jl?
  • License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • Contribution and authorship: Has the submitting author (@SotaYoshida) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  • Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
  • Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
  • Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
  • Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • 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

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

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

@mdavezac
Copy link

The package is a truly ambitious effort to translate a number of nuclear structure many-body methods to Julia, with the goal of providing a modular framework to advance such methods for students and researchers. Unfortunately, the code makes little use of best practices in software design (unit-testing, separation of concerns, encapsulation, etc), Julia's capabilities promoting modularity (multiple dispatch), and reads more like a direct translation of an old Fortran code than a modern implementation. This in itself should not stop publication in JOSS. However, I believe the state of the documentation and the testing is not yet sufficient to verify the functionality claims:

  1. there is no example or test for MPI (the code claims to work on supercomputers)
  2. the integration tests such as they are only check the code runs to completion. They do not check the code computes anything, much less that it computes anything correctly.
  3. from the input, it seems the code generates a binary output file. As far as I can tell, there is no documentation of what it contains or how to read and use the data therein.
  4. the API documentation is too often sparse, especially when one considers some functions listed there take 10-20 arguments with no obvious typing. Since this package is meant as a toolkit for the community, I would recommend (though not require) investing some time into it. In this respect, unit-tests demonstrating usage and expected outputs would have been valuable.

Also, the code will not run as a package (see SotaYoshida/NuclearToolkit.jl#27).

There are a few typos in the paper:

  1. first sentence, "among nucleons, nuclear force" -> "among nucleons, [the] nuclear force"
  2. second sentence, "especial[l]y"

@SotaYoshida
Copy link

@mdavezac
Thank you so much for reviewing and feedbacks.
I am now dealing with your points and documents, test codes, etc.
It may take at least a couple of weeks.

I will make changes in between, but I will ask you to confirm at some point not to bother you repeatedly, so please do not care about the details of the changes until then.

Again, I appreciate you taking the time to review this package.

@rkurchin
Copy link

@SotaYoshida, just wanted to check in on the progress on these updates. How are things going?

@SotaYoshida
Copy link

@rkurchin Thank you for the reminder.
According to the reviewer(@mdavezac)'s comment, I have made dozens of modifications so far.
I will tag v0.1.5 within a couple of days and, then reply to the comments with summary of modifications.

@SotaYoshida
Copy link

@editorialbot set v2.0.1 as version

@editorialbot
Copy link
Collaborator Author

I'm sorry @SotaYoshida, I'm afraid I can't do that. That's something only editors are allowed to do.

@SotaYoshida
Copy link

@mdavezac Apologies for the delay.

I have made dozens of modifications on the package according to your valuable comment, and then tagged the latest one as v2.0.1.
(For the above command , I didn't understand how to interact with the bot. Please do not care.)

Here is the summary and reply to your comment:

  1. For the documantation

    I have made new pages, HowToUse and FileFormat to show example usage of the main APIs and to explain what input/output files are, on the document.

  2. For test codes

    there is no example or test for MPI (the code claims to work on supercomputers)

    Absolutely. An example code and a test are added to the document and CI test.
    I am not fully aware of how much is required in terms of unit testing, but I have added a test code to validate the outputs of many-body methods with a specific parameter set. I think this somewhat improved the mitigation of destructive changes during the code development.

  3. the API documentation is too often sparse, especially when one considers some functions listed there take 10-20 arguments with no obvious typing. Since this package is meant as a toolkit for the community, I would recommend (though not require) investing some time into it. In this respect, unit-tests demonstrating usage and expected outputs would have been valuable.

    Yes. First, I have modified main APIs, especially make_chiEFTint(), and structure of functions therein, and the package is now available to run outside the repository. #27
    Second, I have added detailed docstrings on the main APIs and homemade (mutable) structs, and added annotations on the structs so that a user can trace the workflow of the code by e.g. GUI editor like VScode.

  4. I fixed the typos in the paper

I know you are very busy, but I hope you could give us another review. Thank you so much for your time.

@rkurchin
Copy link

@SotaYoshida thanks for all your work on these changes! Just so you know, adding the version tag via the bot is something only I as the editor have the permission to do. I usually wait until the end of the review to do that so it can just be set when it's finalized. So don't worry about it!

@villaa
Copy link

villaa commented Oct 2, 2022

Review checklist for @villaa

Conflict of interest

  • I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

General checks

  • Repository: Is the source code for this software available at the https://github.com/SotaYoshida/NuclearToolkit.jl?
  • License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • Contribution and authorship: Has the submitting author (@SotaYoshida) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  • Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
  • Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
  • Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
  • Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • 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

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

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

@villaa
Copy link

villaa commented Oct 2, 2022

@SotaYoshida I started reviewing, I think the paper is well written I submitted one issue on GitHub relating to reference links and DOIs for arXiv-only references. I generally tag the issues related to my review with [JOSS Review].

I will continue reviewing the software!

@mdavezac
Copy link

mdavezac commented Oct 3, 2022

@SotaYoshida, thank-you for all your work. I'm afraid I have not been able to get to it yet, but I hope to finish the review by next week.

@SotaYoshida

This comment was marked as duplicate.

@editorialbot
Copy link
Collaborator Author

Done! Archive is now 10.5281/zenodo.7314918

@rkurchin
Copy link

Two more tiny fixes on paper:

  • line 26: should begin "In the nuclear physics community..."
  • you're still missing a space before the citation on line 35

After that we should be good to go!

@SotaYoshida
Copy link

@rkurchin Thanks!
I have fixed the two points. (I made a josspaper branch and then merged to the main branch)

@rkurchin
Copy link

@editorialbot generate pdf

@editorialbot
Copy link
Collaborator Author

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@rkurchin
Copy link

@editorialbot recommend-accept

@editorialbot
Copy link
Collaborator Author

Attempting dry run of processing paper acceptance...

@editorialbot
Copy link
Collaborator Author

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.48550/arXiv.1209.5145 is OK
- 10.48550/arXiv.2208.02464 is OK
- 10.1016/S0146-6410(01)00159-4 is OK
- 10.1103/RevModPhys.77.427 is OK
- 10.1103/RevModPhys.92.015002 is OK
- 10.1146/annurev-nucl-101917-021120 is OK
- 10.1016/j.physrep.2011.02.001 is OK
- 10.1088/0031-8949/91/8/083007 is OK
- 10.1103/RevModPhys.81.1773 is OK
- 10.3389/fphy.2020.00098 is OK
- 10.1140/epja/i2003-10096-0 is OK
- 10.1093/ptep/ptac057 is OK
- 10.1017/CBO9780511596834 is OK
- 10.1016/j.cpc.2013.07.022 is OK
- 10.48550/arXiv.1801.08432 is OK
- 10.1016/j.nds.2014.07.022 is OK
- 10.48550/arXiv.1310.5431 is OK
- 10.1016/j.cpc.2019.06.011 is OK

MISSING DOIs

- None

INVALID DOIs

- None

@editorialbot
Copy link
Collaborator Author

👋 @openjournals/pe-eics, this paper is ready to be accepted and published.

Check final proof 👉📄 Download article

If the paper PDF and the deposit XML files look good in openjournals/joss-papers#3721, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

@editorialbot editorialbot added the recommend-accept Papers recommended for acceptance in JOSS. label Nov 14, 2022
@kyleniemeyer
Copy link

Hi @SotaYoshida, I made a small fix to the .bib file, could you merge that? SotaYoshida/NuclearToolkit.jl#63

@SotaYoshida
Copy link

Hi @kyleniemeyer. Thank you for your modifications.
I have merged your pull request.

@kyleniemeyer
Copy link

@editorialbot generate pdf

@editorialbot
Copy link
Collaborator Author

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@kyleniemeyer
Copy link

@editorialbot accept

@editorialbot
Copy link
Collaborator Author

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

@editorialbot
Copy link
Collaborator Author

🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦

@editorialbot
Copy link
Collaborator Author

🚨🚨🚨 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 👉 Creating pull request for 10.21105.joss.04694 joss-papers#3733
  2. Wait a couple of minutes, then verify that the paper DOI resolves https://doi.org/10.21105/joss.04694
  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...

@editorialbot editorialbot added accepted published Papers published in JOSS labels Nov 18, 2022
@kyleniemeyer
Copy link

Congratulations @SotaYoshida on your article's publication in JOSS!

Many thanks to @mdavezac and @villaa for reviewing this submission, and @rkurchin for editing.

@editorialbot
Copy link
Collaborator Author

🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉

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

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

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

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

This is how it will look in your documentation:

DOI

We need your help!

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Julia published Papers published in JOSS recommend-accept Papers recommended for acceptance in JOSS. review TeX Track: 3 (PE) Physics and Engineering
Projects
None yet
Development

No branches or pull requests

6 participants