openjournals / joss-reviews

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

[REVIEW]: PyZFS: A Python package for first-principles calculations of zero-field splitting tensors #2160

Closed whedon closed 4 years ago

whedon commented 4 years ago

Submitting author: @hema-ted (He Ma) Repository: https://github.com/hema-ted/pyzfs Version: v1.3 Editor: @dfm Reviewer: @xwang862, @malramsay64 Archive: 10.5281/zenodo.3728346

Status

status

Status badge code:

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

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 & @malramsay64, 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.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @dfm know.

Please try and complete your review in the next two weeks

Review checklist for @xwang862

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @malramsay64

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 4 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @xwang862, @malramsay64 it looks like you're currently assigned to review 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

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

@whedon generate pdf
whedon commented 4 years ago
Reference check summary:

OK DOIs

- None

MISSING DOIs

- https://doi.org/10.1002/wcms.81 may be missing for title: The ORCA program system
- https://doi.org/10.1103/physrevb.77.035119 may be missing for title: First principles method for the calculation of zero-field splitting tensors in periodic systems
- https://doi.org/10.1147/rd.521.0137 may be missing for title: Architecture of Qbox: A scalable first-principles molecular dynamics code
- https://doi.org/10.1103/physrevmaterials.1.075002 may be missing for title: Designing defect-based qubit candidates in wide-gap binary semiconductors for solid-state quantum technologies
- https://doi.org/10.1038/s41567-019-0420-0 may be missing for title: Spin-phonon interactions in silicon carbide addressed by Gaussian acoustics
- https://doi.org/10.1103/physrevmaterials.3.043801 may be missing for title: All-electron density functional calculations for electron and nuclear spin interactions in molecules and solids

INVALID DOIs

- None
whedon commented 4 years ago

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

dfm commented 4 years ago

@hema-ted: Please update the DOIs in the paper as suggested by whedon. Thanks!

dfm commented 4 years ago

@xwang862, @malramsay64: Your reviewer instructions and checklists are above and there's more info on the documentation page: https://joss.readthedocs.io/en/latest/index.html

Please don't hesitate to ask if you have any questions throughout this process!

arfon commented 4 years ago

Dear authors and reviewers

We wanted to notify you that in light of the current COVID-19 pandemic, JOSS has decided to suspend submission of new manuscripts and to handle existing manuscripts (such as this one) on a "best efforts basis". We understand that you may need to attend to more pressing issues than completing a review or updating a repository in response to a review. If this is the case, a quick note indicating that you need to put a "pause" on your involvement with a review would be appreciated but is not required.

Thanks in advance for your understanding.

Arfon Smith, Editor in Chief, on behalf of the JOSS editorial team.

malramsay64 commented 4 years ago

Hi @hema-ted and @dfm,

I have had the opportunity to complete my review. The final (very minor) outstanding issue is https://github.com/hema-ted/pyzfs/issues/9 which will likely be addressed very shortly. Thank you @hema-ted for being so quick to respond and address feedback.

dfm commented 4 years ago

@malramsay64: Thanks for your review!

I'll follow up with @xwang862 via email to make sure that he's getting notifications, but we should all be patient given everything @arfon mentions above.

hema-ted commented 4 years ago

@malramsay64,

Thanks a lot for your comments and suggested improvements!

xwang862 commented 4 years ago

Hi @hema-ted , thanks for the great work. Both the codes and the summary paper are well structured and clearly written. Most of my comments are trying to increase the accessibility of your package to general users, as well as to satisfy my own curiosity. I will mention them in the order of the checklist:

hema-ted commented 4 years ago

Hi @xwang862,

Thanks for your comments and suggestions! I will address some of them here and get back to the rest of them shortly.

help flag: I have included the --help flag (commit e03f6e33).

fftgrid and memory options: I have added more detailed explanations for them in the tutorial (commit 36bccf1b).

wavefunction files: I have included all the input files for DFT codes (QE, Qbox) in my examples. There is a run.sh file in each folder of the /examples directory, which includes the command to perform corresponding DFT calculations to obtain the wavefunction.

hema-ted commented 4 years ago

Hi @xwang862,

Thanks again for your comments. I addressed them in several recent commits. See a summary below.

General checks: I added a , I also expanded the README.md file about the scope of the code.

Documentation - comparison to other codes: Currently, we are not aware of other popular open-sourced code for this type of calculation in the plane-wave basis.

Documentation - output file: I included a short description of the output file by the end of the tutorial.

Paper - O2 and spin-orbital coupling: Indeed, the spin-orbital contribution to O2 is on the same order of magnitude as the spin-spin contribution. It is a general trend that the spin-spin contribution to ZFS tensor dominates the spin-orbit contribution for molecules and materials with light main group elements, and O2 is an (interesting) exception rather than a common case (for example, see https://pubs.acs.org/doi/10.1021/jp0643303 for the calculation of organic molecules, where spin-spin contribution dominates). We used O2 as an example to demonstrate the use of the code because O2 is one of the smallest molecules with a spin-triplet ground state, and the value of its ZFS tensor (both contributions) is well known. Also, for many defects in semiconductors with high point group symmetry, the spin-orbit contributions to the ZFS tensor are zero, and only spin-spin contributions present.

Paper - a reference for eq. 2: I added a reference for eq. 2.

Paper - comparison to other codes like ORCA: PyZFS focuses on computing ZFS tensor using plane-wave basis sets while ORCA uses atomic basis sets. The comparison between ZFS tensors computed using different basis sets is beyond the scope of the current paper, but there are papers (e.g. https://journals.aps.org/prmaterials/abstract/10.1103/PhysRevMaterials.3.043801) that address this issue. The reference values provided in the examples are provided for automatic testing and for verification of installation, and the values are obtained using PyZFS itself.

Paper - timing and parallelization: I added a brief description by the end of the tutorial for timing and parallelization.

Please let me know if you have any further comments.

Finally, I gratefully added the acknowledgment to both reviewers in the paper.

xwang862 commented 4 years ago

Thanks @hema-ted for your effort and thorough responses! My comments have been appropriately addressed.

dfm commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

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

dfm commented 4 years ago

@whedon check references

whedon commented 4 years ago
Reference check summary:

OK DOIs

- None

MISSING DOIs

- https://doi.org/10.1002/wcms.81 may be missing for title: The ORCA program system
- https://doi.org/10.1103/physrevb.77.035119 may be missing for title: First principles method for the calculation of zero-field splitting tensors in periodic systems
- https://doi.org/10.1147/rd.521.0137 may be missing for title: Architecture of Qbox: A scalable first-principles molecular dynamics code
- https://doi.org/10.1103/physrevmaterials.1.075002 may be missing for title: Designing defect-based qubit candidates in wide-gap binary semiconductors for solid-state quantum technologies
- https://doi.org/10.1038/s41567-019-0420-0 may be missing for title: Spin-phonon interactions in silicon carbide addressed by Gaussian acoustics
- https://doi.org/10.1103/physrevmaterials.3.043801 may be missing for title: All-electron density functional calculations for electron and nuclear spin interactions in molecules and solids
- https://doi.org/10.1016/c2013-0-06187-7 may be missing for title: Theoretical foundations of electron spin resonance

INVALID DOIs

- None
dfm commented 4 years ago

@hema-ted: Can you fix the DOIs as suggested by whedon? I'll have some more comments on the paper shortly.

dfm commented 4 years ago

Actually, the paper looks great to me. So, after you add the DOIs, can you bump the version number, tag a new release on GitHub, and archive that release using Zenodo or similar. Please make sure that the archive has the same title and author list as the paper (you might need to edit the metadata if the release is automatically generated). Once you've made these updates, please report the version number and Zenodo DOI here and we'll proceed. Thanks!

hema-ted commented 4 years ago

@dfm I have added the DOIs, changed the version number (1.3) and tagged a new release (named 'joss'), the Zenodo DOI for the release is 10.5281/zenodo.3728346. Please let me know if everything looks right. Thanks!

dfm commented 4 years ago

Thanks! Can you please edit the Zenodo metadata so that the author list and title match the paper?

hema-ted commented 4 years ago

@dfm I have changed the metadata and now it looks consistent with the paper.

dfm commented 4 years ago

@whedon set 10.5281/zenodo.3728346 as archive

whedon commented 4 years ago

OK. 10.5281/zenodo.3728346 is the archive.

dfm commented 4 years ago

@whedon set v1.3 as version

whedon commented 4 years ago

OK. v1.3 is the version.

dfm commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

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

dfm commented 4 years ago

@whedon accept

whedon commented 4 years ago
Attempting dry run of processing paper acceptance...
whedon commented 4 years ago
Reference check summary:

OK DOIs

- 10.1002/wcms.81 is OK
- 10.1103/PhysRevB.77.035119 is OK
- 10.1088/0953-8984/21/39/395502 is OK
- 10.1147/rd.521.0137 is OK
- 10.1103/physrevmaterials.1.075002 is OK
- 10.1038/s41567-019-0420-0 is OK
- 10.1103/physrevmaterials.3.043801 is OK

MISSING DOIs

- https://doi.org/10.1016/c2013-0-06187-7 may be missing for title: Theoretical foundations of electron spin resonance

INVALID DOIs

- None
whedon commented 4 years ago

:wave: @openjournals/joss-eics, this paper is ready to be accepted and published.

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

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

@whedon accept deposit=true
dfm commented 4 years ago

@hema-ted: Sorry - I missed that last DOI. Can you please add that to your .bib file? Also: please double check the final proofs above to make sure that everything looks good to you. The editors-in-chief team will take a look and do the final processing when they get a chance.

dfm commented 4 years ago

@xwang862, @malramsay64: Thank you very much for your constructive reviews! We really appreciate your contributions.

danielskatz commented 4 years ago

👋 @hema-ted & @dfm - since it's not so useful to have multiple people proof-reading at the same time, please let us (@openjournals/jose-eics) know when you think this is ready to publish, then we will take a final look and move it forward.

hema-ted commented 4 years ago

@openjournals/jose-eics I have proofread the final version and I think it is ready for publication. Thanks!

dfm commented 4 years ago

That should be @openjournals/joss-eics! This is ready to go. Thanks!

danielskatz commented 4 years ago

thanks - we'll take it from here

danielskatz commented 4 years ago

Please see changes in https://github.com/hema-ted/pyzfs/pull/12 and https://github.com/hema-ted/pyzfs/pull/13

hema-ted commented 4 years ago

@danielskatz Thanks, I have merged the pull requests.

danielskatz commented 4 years ago

@whedon accept

whedon commented 4 years ago
Attempting dry run of processing paper acceptance...
whedon commented 4 years ago
Reference check summary:

OK DOIs

- 10.1002/wcms.81 is OK
- 10.1103/PhysRevB.77.035119 is OK
- 10.1088/0953-8984/21/39/395502 is OK
- 10.1147/rd.521.0137 is OK
- 10.1103/physrevmaterials.1.075002 is OK
- 10.1038/s41567-019-0420-0 is OK
- 10.1103/physrevmaterials.3.043801 is OK

MISSING DOIs

- None

INVALID DOIs

- https://doi.org/10.1016/c2013-0-06187-7 is INVALID because of 'https://doi.org/' prefix
whedon commented 4 years ago

:wave: @openjournals/joss-eics, this paper is ready to be accepted and published.

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

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

@whedon accept deposit=true
danielskatz commented 4 years ago

👋 @hema-ted - sorry, I now see a DOI that needs to be fixed, as done in https://github.com/hema-ted/pyzfs/pull/14

danielskatz commented 4 years ago

@whedon accept

whedon commented 4 years ago
Attempting dry run of processing paper acceptance...
whedon commented 4 years ago
Reference check summary:

OK DOIs

- 10.1002/wcms.81 is OK
- 10.1103/PhysRevB.77.035119 is OK
- 10.1088/0953-8984/21/39/395502 is OK
- 10.1147/rd.521.0137 is OK
- 10.1103/physrevmaterials.1.075002 is OK
- 10.1038/s41567-019-0420-0 is OK
- 10.1103/physrevmaterials.3.043801 is OK
- 10.1016/c2013-0-06187-7 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 4 years ago

:wave: @openjournals/joss-eics, this paper is ready to be accepted and published.

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

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

@whedon accept deposit=true
danielskatz commented 4 years ago

@whedon accept deposit=true