openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
717 stars 38 forks source link

[REVIEW]: Atomic Simulation Interface (ASI): application programming interface for electronic structure codes #5186

Closed editorialbot closed 1 year ago

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@PavelStishenko<!--end-author-handle-- (Pavel Stishenko) Repository: https://gitlab.com/pvst/asi Branch with paper.md (empty if default branch): master Version: 1.1.0 Editor: !--editor-->@rkurchin<!--end-editor-- Reviewers: @xwang862, @junghans, @srmnitc Archive: 10.5281/zenodo.7931108

Status

status

Status badge code:

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

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

@xwang862 & @junghans & @srmnitc, 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 @xwang862

📝 Checklist for @junghans

📝 Checklist for @srmnitc

editorialbot commented 1 year ago

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 commented 1 year ago
Software report:

github.com/AlDanial/cloc v 1.88  T=0.08 s (911.0 files/s, 68823.1 lines/s)
---------------------------------------------------------------------------------------
Language                             files          blank        comment           code
---------------------------------------------------------------------------------------
C++                                      8            229             35           1357
Python                                  15            249            306           1052
TeX                                      1             42              1            350
Bourne Shell                            28            141              9            294
C/C++ Header                             4            116            190            185
Markdown                                 3            100              0            174
YAML                                     2             12              4            169
CMake                                    3             76            169             95
reStructuredText                         5             72             74             74
make                                     3             27              7             64
Windows Module Definition                3             20              0             62
DOS Batch                                1              8              1             26
TOML                                     1              2              0             25
---------------------------------------------------------------------------------------
SUM:                                    77           1094            796           3927
---------------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 1 year ago

Wordcount for paper.md is 2196

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

editorialbot commented 1 year ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1088/1361-648x/aa680e is OK
- 10.1021/acs.jctc.8b01036 is OK
- 10.1063/5.0012901 is OK
- 10.1103/PhysRevX.8.041048 is OK
- 10.1021/acs.jctc.8b00908 is OK
- 10.1103/PhysRevLett.114.096405 is OK
- 10.1126/science.aag2302 is OK
- 10.1021/acs.jctc.8b00873 is OK
- 10.1038/s41467-019-12875-2 is OK
- 10.1063/5.0004608 is OK
- 10.1063/5.0005082 is OK
- 10.1063/5.0007045 is OK
- 10.1063/1.5143190 is OK
- 10.1103/PhysRevB.71.035109 is OK
- 10.21105/jcon.00069 is OK
- 10.1021/ct400698y is OK
- 10.1021/acs.jctc.1c00751 is OK
- 10.1038/s41524-022-00843-2 is OK

MISSING DOIs

- 10.1016/j.cpc.2017.09.007 may be a valid DOI for title: ELSI: A unified software interface for Kohn–Sham electronic structure solvers

INVALID DOIs

- https://doi.org/10.1016/j.cpc.2020.107459 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1016/j.cpc.2012.05.007 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1016/j.softx.2017.11.002 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1016/j.cpc.2009.06.022 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1002/wcms.93 is INVALID because of 'https://doi.org/' prefix
rkurchin commented 1 year ago

@PavelStishenko, please fix the DOI issues above whenever you have a chance (not urgent to get review started, but will be necessary before we can accept the submission).

Reviewers @xwang862, @junghans, @srmnitc, please let me know if you have any questions about how to proceed!

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

editorialbot commented 1 year ago

Hello @PavelStishenko, here are the things you can ask me to do:


# List all available commands
@editorialbot commands

# Get a list of all editors's GitHub handles
@editorialbot list editors

# Check the references of the paper for missing DOIs
@editorialbot check references

# Perform checks on the repository
@editorialbot check repository

# Adds a checklist for the reviewer using this command
@editorialbot generate my checklist

# Set a value for branch
@editorialbot set joss-paper as branch

# Generates the pdf paper
@editorialbot generate pdf

# Generates a LaTeX preprint file
@editorialbot generate preprint

# Get a link to the complete list of reviewers
@editorialbot list reviewers
editorialbot commented 1 year ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1088/1361-648x/aa680e is OK
- 10.1021/acs.jctc.8b01036 is OK
- 10.1063/5.0012901 is OK
- 10.1016/j.cpc.2017.09.007 is OK
- 10.1016/j.cpc.2020.107459 is OK
- 10.1016/j.cpc.2012.05.007 is OK
- 10.1016/j.softx.2017.11.002 is OK
- 10.1103/PhysRevX.8.041048 is OK
- 10.1021/acs.jctc.8b00908 is OK
- 10.1103/PhysRevLett.114.096405 is OK
- 10.1126/science.aag2302 is OK
- 10.1021/acs.jctc.8b00873 is OK
- 10.1038/s41467-019-12875-2 is OK
- 10.1063/5.0004608 is OK
- 10.1063/5.0005082 is OK
- 10.1063/5.0007045 is OK
- 10.1016/j.cpc.2009.06.022 is OK
- 10.1063/1.5143190 is OK
- 10.1103/PhysRevB.71.035109 is OK
- 10.1002/wcms.93 is OK
- 10.21105/jcon.00069 is OK
- 10.1021/ct400698y is OK
- 10.1021/acs.jctc.1c00751 is OK
- 10.1038/s41524-022-00843-2 is OK

MISSING DOIs

- None

INVALID DOIs

- None
rkurchin commented 1 year ago

Hi @xwang862, @junghans, @srmnitc, reminder to get this review started whenever you can!

xwang862 commented 1 year ago

@rkurchin Sorry for the delay, I'll try to get this review done by the end of next week.

xwang862 commented 1 year ago

Review checklist for @xwang862

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

junghans commented 1 year ago

Review checklist for @junghans

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

junghans commented 1 year ago

Re: contributing to the ASI see https://gitlab.com/pvst/asi/-/issues/2

srmnitc commented 1 year ago

Review checklist for @srmnitc

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

junghans commented 1 year ago

Re: contributing to the ASI see https://gitlab.com/pvst/asi/-/issues/2

Fixed

xwang862 commented 1 year ago

@PavelStishenko thanks for the nice work. I find this software useful and the summary paper clearly written. A few comments below:

rkurchin commented 1 year ago

@PavelStishenko, just checking in on this!

PavelStishenko commented 1 year ago

@PavelStishenko, just checking in on this!

@rkurchin We are working on the @xwang862 suggestions. Aiming to complete and respond on the next week.

PavelStishenko commented 1 year ago

@xwang862 , thanks for the review! The answers are in the quotes below:

* I had an [installation error](https://gitlab.com/pvst/asi/-/issues/4). Could be my own issue, but please address it.

Thanks for helping with debugging that issue. It turns out, that the MacOS support is not currently not available. Adding of the MacOS support is a future work. The list of currently supported and tested platforms has been added to the documentation.

* Please add more details in your [build instructions](https://gitlab.com/pvst/asi#building). For example, to correctly build DFTB+, one needs to clone your forked repo, switch to branch `api-H-import`, follow the "building from source" method, etc. To set environment variables, should one just copy and modify your `envs` examples? Which variables are necessary and which are optional?

The documentation of the building process has been significantly extended. Necessary and optional environment variables have been listed and described. The building scripts have been refactored aiming to simplicity. The envs directory is removed, because the new documentation has necessary examples and supersedes it.

* Please add explanations/comments to help users better understand your [usage example](https://pvst.gitlab.io/asi/intro.html#usage-example).

Comments have been added into the code of the usage example.

* Please add documentation to the tests (in `tests/` dir), as many people would learn by adapting those test files. Although the filenames indicate their usage to some extent, it would be helpful if you add descriptions within each file, e.g. what does a test do, which DFT code is used, purposes of functions/code blocks, etc.

The documentation has been added for tests building and running. Descriptions have been added for each test.

xwang862 commented 1 year ago

Thanks @PavelStishenko for your extensive responses. My comments have been appropriately addressed. 👍

rkurchin commented 1 year ago

Thanks, @xwang862. Checking in on @srmnitc's review progress?

rkurchin commented 1 year ago

🔔 Pinging @srmnitc again – the other two reviews are finished, so we're just waiting on you here!

srmnitc commented 1 year ago

@rkurchin I apologize the delay from my side due to some other commitements. I will finish the review on by 02.05. I hope this is ok.

srmnitc commented 1 year ago

@rkurchin @PavelStishenko I have now finished my review. I only have few minor points about the paper for which I have started this issue. Apart from these, this submission is complete and could be useful for the community.

rkurchin commented 1 year ago

@editorialbot generate post-review checklist

Thanks everyone for your help on this review! The command above will generate a checklist to keep track of the last few things we need to take care of before accepting this submission.

editorialbot commented 1 year ago

I'm sorry human, I don't understand that. You can see what commands I support by typing:

@editorialbot commands

rkurchin commented 1 year ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

rkurchin commented 1 year ago

@editorialbot check references

editorialbot commented 1 year ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1088/1361-648x/aa680e is OK
- 10.1021/acs.jctc.8b01036 is OK
- 10.1063/5.0012901 is OK
- 10.1016/j.cpc.2017.09.007 is OK
- 10.1016/j.cpc.2020.107459 is OK
- 10.1016/j.cpc.2012.05.007 is OK
- 10.1016/j.softx.2017.11.002 is OK
- 10.1103/PhysRevX.8.041048 is OK
- 10.1021/acs.jctc.8b00908 is OK
- 10.1103/PhysRevLett.114.096405 is OK
- 10.1126/science.aag2302 is OK
- 10.1021/acs.jctc.8b00873 is OK
- 10.1038/s41467-019-12875-2 is OK
- 10.1063/5.0004608 is OK
- 10.1063/5.0005082 is OK
- 10.1063/5.0007045 is OK
- 10.1016/j.cpc.2009.06.022 is OK
- 10.1063/1.5143190 is OK
- 10.1103/PhysRevB.71.035109 is OK
- 10.1002/wcms.93 is OK
- 10.21105/jcon.00069 is OK
- 10.1021/ct400698y is OK
- 10.1021/acs.jctc.1c00751 is OK
- 10.1038/s41524-022-00843-2 is OK

MISSING DOIs

- None

INVALID DOIs

- None
rkurchin commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

rkurchin commented 1 year ago

A few small editorial comments (thanks also to @srmnitc for his in the aforementioned issue):

A few conceptual comments also since this is close enough to my own area of expertise that I feel comfortable making them 😄 :

Please also do the checks suggested in the checklist above and send me the info (version number, DOI) to attach to this submission. We're almost there! 🚀

PavelStishenko commented 1 year ago

@editorialbot generate pdf

PavelStishenko commented 1 year ago

@editorialbot check references

editorialbot commented 1 year ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1088/1361-648x/aa680e is OK
- 10.1021/acs.jctc.8b01036 is OK
- 10.1063/5.0012901 is OK
- 10.1016/j.cpc.2017.09.007 is OK
- 10.1016/j.cpc.2020.107459 is OK
- 10.1016/j.cpc.2012.05.007 is OK
- 10.1016/j.softx.2017.11.002 is OK
- 10.1103/PhysRevX.8.041048 is OK
- 10.1021/acs.jctc.8b00908 is OK
- 10.1103/PhysRevLett.114.096405 is OK
- 10.1126/science.aag2302 is OK
- 10.1021/acs.jctc.8b00873 is OK
- 10.1038/s41467-019-12875-2 is OK
- 10.1063/5.0004608 is OK
- 10.1063/5.0005082 is OK
- 10.1063/5.0007045 is OK
- 10.1016/j.cpc.2009.06.022 is OK
- 10.1063/1.5143190 is OK
- 10.1103/PhysRevB.71.035109 is OK
- 10.1002/wcms.93 is OK
- 10.21105/jcon.00069 is OK
- 10.1021/ct400698y is OK
- 10.1021/acs.jctc.1c00751 is OK
- 10.1038/s41524-022-00843-2 is OK
- 10.1038/s41586-020-2649-2 is OK
- 10.1021/acs.jctc.1c00834 is OK

MISSING DOIs

- None

INVALID DOIs

- doi.org/10.1016/j.cpc.2020.107688 is INVALID because of 'doi.org/' prefix
editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

PavelStishenko commented 1 year ago

@editorialbot check references

editorialbot commented 1 year ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1088/1361-648x/aa680e is OK
- 10.1021/acs.jctc.8b01036 is OK
- 10.1063/5.0012901 is OK
- 10.1016/j.cpc.2017.09.007 is OK
- 10.1016/j.cpc.2020.107459 is OK
- 10.1016/j.cpc.2012.05.007 is OK
- 10.1016/j.softx.2017.11.002 is OK
- 10.1103/PhysRevX.8.041048 is OK
- 10.1021/acs.jctc.8b00908 is OK
- 10.1103/PhysRevLett.114.096405 is OK
- 10.1126/science.aag2302 is OK
- 10.1021/acs.jctc.8b00873 is OK
- 10.1038/s41467-019-12875-2 is OK
- 10.1063/5.0004608 is OK
- 10.1063/5.0005082 is OK
- 10.1063/5.0007045 is OK
- 10.1016/j.cpc.2009.06.022 is OK
- 10.1063/1.5143190 is OK
- 10.1103/PhysRevB.71.035109 is OK
- 10.1002/wcms.93 is OK
- 10.21105/jcon.00069 is OK
- 10.1021/ct400698y is OK
- 10.1021/acs.jctc.1c00751 is OK
- 10.1038/s41524-022-00843-2 is OK
- 10.1038/s41586-020-2649-2 is OK
- 10.1016/j.cpc.2020.107688 is OK
- 10.1021/acs.jctc.1c00834 is OK

MISSING DOIs

- None

INVALID DOIs

- None
PavelStishenko commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

PavelStishenko commented 1 year ago

@srmnitc , thanks for the review. Your comments in the issue were addressed and necessary correction has been introduced.

@rkurchin , thanks for your notes. All suggested corrections has been introduced. Thanks for the conceptual comments. The MolSSI Driver Interface is really related and should be mentioned here. The appropriate citation also was added. And yes, indeed, the ASI API really fits to facilitate AIMD workflows. The corresponding additions has been made.

rkurchin commented 1 year ago

@editorialbot check references

editorialbot commented 1 year ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1088/1361-648x/aa680e is OK
- 10.1021/acs.jctc.8b01036 is OK
- 10.1063/5.0012901 is OK
- 10.1016/j.cpc.2017.09.007 is OK
- 10.1016/j.cpc.2020.107459 is OK
- 10.1016/j.cpc.2012.05.007 is OK
- 10.1016/j.softx.2017.11.002 is OK
- 10.1103/PhysRevX.8.041048 is OK
- 10.1021/acs.jctc.8b00908 is OK
- 10.1103/PhysRevLett.114.096405 is OK
- 10.1126/science.aag2302 is OK
- 10.1021/acs.jctc.8b00873 is OK
- 10.1038/s41467-019-12875-2 is OK
- 10.1063/5.0004608 is OK
- 10.1063/5.0005082 is OK
- 10.1063/5.0007045 is OK
- 10.1016/j.cpc.2009.06.022 is OK
- 10.1063/1.5143190 is OK
- 10.1103/PhysRevB.71.035109 is OK
- 10.1002/wcms.93 is OK
- 10.21105/jcon.00069 is OK
- 10.1021/ct400698y is OK
- 10.1021/acs.jctc.1c00751 is OK
- 10.1038/s41524-022-00843-2 is OK
- 10.1038/s41586-020-2649-2 is OK
- 10.1016/j.cpc.2020.107688 is OK
- 10.1021/acs.jctc.1c00834 is OK

MISSING DOIs

- None

INVALID DOIs

- None
rkurchin commented 1 year ago

Great! So to finish this up, you just need to help me finish the checklist above – in particular, the items under the "author tasks" header.

PavelStishenko commented 1 year ago

Here are: version: 1.1.0 DOI: 10.5281/zenodo.7931108

rkurchin commented 1 year ago

@editorialbot set 1.1.0 as version

editorialbot commented 1 year ago

Done! version is now 1.1.0

rkurchin commented 1 year ago

@PavelStishenko it looks like the archive has a CCA4.0 license while the repo has an MIT license. Can we make those match up?

PavelStishenko commented 1 year ago

@PavelStishenko it looks like the archive has a CCA4.0 license while the repo has an MIT license. Can we make those match up?

Fixed!