[REVIEW]: Surfaxe: Systematic surface calculations

[whedon]

Submitting author: @brlec (Katarina Brlec)
Repository: https://github.com/SMTG-UCL/surfaxe
Version: v1.2
Editor: @danielskatz
Reviewer: @pzarabadip, @eihernan
Archive: 10.5281/zenodo.4776449

⚠️ JOSS reduced service mode ⚠️

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

Status badge code:

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

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

@pzarabadip & @eihernan, 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 @danielskatz 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 ✨

Review checklist for @pzarabadip

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

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

General checks

• Repository: Is the source code for this software available at the repository url?
• 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 (@brlec) 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

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 and who the target audience is?
• 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?

Review checklist for @eihernan

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

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

General checks

• Repository: Is the source code for this software available at the repository url?
• 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 (@brlec) 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

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 and who the target audience is?
• 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?

[whedon]

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @pzarabadip, @eihernan it looks like you're currently assigned to review this paper 🎉.

⚠️ JOSS reduced service mode ⚠️

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

⭐ Important ⭐

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:

2. 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

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

@whedon generate pdf

[whedon]

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

OK DOIs

- 10.1002/cpe.3505 is OK
- 10.1016/j.commatsci.2012.10.028 is OK
- 10.1088/1361-648X/aa680e is OK
- 10.21105/joss.01944 is OK
- 10.1016/j.commatsci.2020.110086 is OK

MISSING DOIs

- None

INVALID DOIs

- None

[whedon]

Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=37.05 s (1.9 files/s, 32345.6 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
XML                              8              0              0        1191898
Python                          27            629            936           2272
JSON                             3              0              0            360
reStructuredText                10             74            106            147
Markdown                         2             72              0            145
YAML                            13              4              0            117
Jupyter Notebook                 4              0           1436            102
TeX                              1              3              0             78
DOS Batch                        1              8              1             26
make                             1              4              7              9
-------------------------------------------------------------------------------
SUM:                            70            794           2486        1195154
-------------------------------------------------------------------------------


Statistical information for the repository '18472d3ed532dce818f85b45' was
gathered on 2021/04/14.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Dan Davies                       3            93              2            0.91
Daniel Davies                   29           200             96            2.83
Sean Kavanagh                    1             1              1            0.02
brlec                          104          6685           3376           96.24

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
Dan Davies                  214          230.1          4.1               16.36
brlec                      3623           54.2          2.9                7.09

[whedon]

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

[danielskatz]

👋 @pzarabadip and @eihernan - Thanks for agreeing to review this submission.
This is the review thread for the paper. All of our communications will happen here from now on.

Both reviewers have checklists at the top of this thread with the JOSS requirements. As you go over the submission, please check any items that you feel have been satisfied. There are also links to the JOSS reviewer guidelines.

Please read the first couple of comments in this issue carefully, so that you can accept the invitation from JOSS and be able to check items, and so that you don't get overwhelmed with notifications from other activities in JOSS.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention openjournals/joss-reviews#3171 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for reviews to be completed within about 2-4 weeks. Please let me know if either of you require some more time. We can also use Whedon (our bot) to set automatic reminders if you know you'll be away for a known period of time.

Please feel free to ping me (@danielskatz) if you have any questions/concerns.

[whedon]

👋 @pzarabadip, please update us on how your review is going (this is an automated reminder).

[whedon]

👋 @eihernan, please update us on how your review is going (this is an automated reminder).

[danielskatz]

👋 @pzarabadip & @eihernan - How are your reviews going?

[ezpzbz]

Hi @danielskatz.
Sorry for late response. I just finished my review on the code.

I would like to thank authors for developing surfaxe package which is nice and useful contribution for the community.
I carefully went through code, tutorials, tests, documentation, and paper and found from my point view that the code/paper is (almost) ready to be accepted for publication in JOSS. I only have few suggestions that could be nice to be implemented before publication.

• Applying formatting/linting tools on the code for which I opened an issue and explained it in more detail.
• Another suggestion would be moving the CI to GitHub Actions to improve the sustainability in future.
• From my personal experience: it would be beneficial for the performance to use OUTCAR parser of parsevasp package instead of the one provided by pymatgen.

Best regards,
Pezhman

[danielskatz]

Thanks @pzarabadip - Do I understand correctly that these are suggestions, not changes that are required for you?

Or is the last one a requirement, tied to the fact that you didn't check the performance box in your review?

[danielskatz]

👋 @eihernan - Can you update us on the status of your review?

[ezpzbz]

Hi @danielskatz - That's correct, those are suggestions. I missed to check the performance box. The last one can improve the performance for large files but nothing against current performance of the code.

[eihernan]

Hi @danielskatz, I've been a bit behind on this. I'll update with progress shortly.

[danielskatz]

ok - it's great to hear from you that you're still workin on this, and we'll look forward to a further update

[brlec]

Hi @pzarabadip,

Thank you for the thorough review!

To answer the suggestions:

• Applying formatting/linting tools: We have removed the unused imports in the latest commit on the develop branch and they will be merged with master once JOSS review is complete. We have added linting tools that should catch unused imports going forward.
• Moving the CI to GitHub Actions: We have implemented this in the latest commit on the master branch, the tests are passing.
• Using parsevasp: OUTCARs are parsed for two different purposes in surfaxe - getting timing information from convergence calculations or getting the core level energies. We have tested parsevasp vs pymatgen and found that parsevasp offers a 10x speedup for reading the timing information.

from parsevasp import outcar
from pymatgen.io.vasp.outputs import Outcar
import time

# pymatgen
start = time.time()
otc = Outcar('surfaxe/example_data/convergence/Y2Ti2S2O5/001/30_50_15/OUTCAR')
times = otc.run_stats
print(time.time() - start)
>> 0.21026110649108887

# parsevasp
start = time.time()
otc = outcar.Outcar(file_path='surfaxe/example_data/convergence/Y2Ti2S2O5/001/30_50_15/OUTCAR')
times = otc.get_run_stats()
print(time.time() -  start)
>> 0.029942989349365234

However, the additional wait time for the user arising from pymatgen parser would be minimal as parsing of vasprun.xml takes far longer. parsevasp OUTCAR parser also cannot read in the core level energies (yet), so we are going to stick with pymatgen as the package for reading in OUTCARs. We will definitely look into using parsevasp in the future if parsing of core level energies is added.

Thanks again for the really helpful review!
Katarina

[eihernan]

Hi @danielskatz, here is an update of my review status.

I have gone through the docs, tutorials, and paper and find them to be clear in their purpose, well-written and easy to follow. The inclusion of a binder environment is appreciated and the notebooks are well documented to explain the limitations when testing on that platform. The tutorials illustrate the functionality of the package well.

I've been able to install and use the package on my personal computer following the installation instructions, but ran into an issue with the CLI not working when installing in a virtual environment. I want to make sure this was (my) user error before I check off the installation box.

Thank you to the authors for their work, I think this package goes a long way to making surface calculation workflows more robust and reproducible. A suggestion for now:

• The package does rely on pymatgen to generate the VASP input (which is fine), however, it's not obvious from the documentation if the package is fully functional without setting up pymatgen to run with VASP or that you can customize its behavior. In particular, my group would not be able to use the make_input_files feature out-of-the-box because we don't keep a central repository of POTCAR files for use with pymatgen. I anticipate users who create their files locally and then run their jobs remotely would encounter this problem as well.
• Along the note above, while the YAML documentation is clear, the generation of the configuration dictionaries would do well to be documented, or at least point to the relevant pymatgen documentation.

Best,
Erick

[danielskatz]

Thanks @eihernan - re the installation problem, if it does continue, you might open an issue in https://github.com/SMTG-UCL/surfaxe and mention https://github.com/openjournals/joss-reviews/issues/3171 there so we can track it.

And I'll look forward to a response from the authors to your other suggestions.

[brlec]

Hi @eihernan,

Thank you for your review and kind words.

Thank you for raising the issue with CLI - we believe this was related to how yaml was imported in the CLI scripts. We have replaced yaml with ruamel.yaml which should work now even in blank virtual environments as ruamel.yaml is a dependency of pymatgen.

To respond to suggestions:

• POTCAR set up for pymatgen: Pymatgen is fairly rigid with respect to how it can read input files, unfortunately this means the POTCARs need to be additionally set up to work with pymatgen. In response to the confusion this may cause we have added the following line to README, with a link to the pymatgen documentation that outlines how to complete the set up.

"For the code to generate VASP input files along with the surface slabs, POTCARs need to be set up with pymatgen"

We also added a similar line to the Installation section of surfaxe docs.

Additionally, we modified the code to check that PMG_VASP_PSP_DIR key exists in pymatgen.core.SETTINGS
before structures are written to file. If POTCARs are not set up, only the structures are written to file and the following warning is raised: POTCAR directory not set up in pymatgen, only POSCARs were generated. We are open to suggestions on how this could be further improved.

• Configuration dictionaries: We agree that for someone not familiar with pymatgen ecosystem this may be somewhat confusing. We added a section in the Tutorials section of documentation on the develop branch outlining how this should be used.

All changes were made on the develop branch if you would like to try re-installing the package in a virtual environment to see if the CLI problem is fixed now?

Thank you again,
Katarina

[eihernan]

Hi @brlec,
I think the additions to the documentation are clear on what a user should do to fully generate VASP inputs. Since the functionality this package provides (creating appropriate file-structures and slabs) is maintained, and this is generally code-agnostic, I think this is a fine solution.

I installed the develop branch, and the CLI is working as expected in a virtual environment.

@danielskatz with the changes above and installation working as expected, I believe this meets the standards for inclusion in JOSS.

Thank you again to the developers.

[danielskatz]

Thanks @eihernan!

[danielskatz]

@brlec - at this point (or once you are ready if there are any further changes you want to make, such as in response to reviewer suggestions), can you:

• Make a tagged release of your software, and list the version tag of the archived version here.
• Archive the reviewed software in Zenodo or a similar service (e.g., figshare, an institutional repository)
• Check the archival deposit (e.g., in Zenodo) has the correct metadata. This includes the title (should match the paper title) and author list (make sure the list is correct and people who only made a small fix are not on it). You may also add the authors' ORCID.
• Please list the DOI of the archived version here.

I can then move forward with accepting the submission, which will include a proof read.

[brlec]

Hi @danielskatz - the JOSS release is tagged v1.2 and was archived on Zenodo with the correct metadata. The DOI is 10.5281/zenodo.4776449

[danielskatz]

@whedon set v1.2 as version

[whedon]

OK. v1.2 is the version.

[danielskatz]

@whedon set 10.5281/zenodo.4776449 as archive

[whedon]

OK. 10.5281/zenodo.4776449 is the archive.

[danielskatz]

@whedon accept

[whedon]

Attempting dry run of processing paper acceptance...

[whedon]

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

OK DOIs

- 10.1002/cpe.3505 is OK
- 10.1016/j.commatsci.2012.10.028 is OK
- 10.1088/1361-648X/aa680e is OK
- 10.21105/joss.01944 is OK
- 10.1016/j.commatsci.2020.110086 is OK

MISSING DOIs

- None

INVALID DOIs

- None

[whedon]

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

Check final proof 👉 openjournals/joss-papers#2328

If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#2328, 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]

@brlec - I'm suggesting some small changes to the paper in SMTG-Bham/surfaxe#11

Please either merge this or let me know what you disagree with.

[brlec]

@danielskatz we've merged all the suggestions

[danielskatz]

@whedon accept

[whedon]

Attempting dry run of processing paper acceptance...

[whedon]

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

OK DOIs

- 10.1002/cpe.3505 is OK
- 10.1016/j.commatsci.2012.10.028 is OK
- 10.1088/1361-648X/aa680e is OK
- 10.21105/joss.01944 is OK
- 10.1016/j.commatsci.2020.110086 is OK

MISSING DOIs

- None

INVALID DOIs

- None

[whedon]

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

Check final proof 👉 openjournals/joss-papers#2329

If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#2329, 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]

@whedon accept deposit=true

[whedon]

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

[whedon]

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

[whedon]

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

Here's what you must now do:

0. Check final PDF and Crossref metadata that was deposited 👉 Creating pull request for 10.21105.joss.03171 joss-papers#2330
1. Wait a couple of minutes, then verify that the paper DOI resolves https://doi.org/10.21105/joss.03171
2. If everything looks good, then close this review issue.
3. Party like you just published a paper! 🎉🌈🦄💃👻🤘

Any issues? Notify your editorial technical team...

[danielskatz]

Congratulations to @brlec (Katarina Brlec) and co-authors!!

And thanks to @pzarabadip and @eihernan for reviewing.

Everyone, this was a really smooth and easy process - thanks again!

[whedon]

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

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

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

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: https://joss.theoj.org/reviewer-signup.html
• Making a small donation to support our running costs here: https://numfocus.org/donate-to-joss