[REVIEW]: MD-SAPT: Python Based Toolkit for Running Symmetry Adapted Perturbation Theory Calculations on Molecular Dynamics Trajectories

[editorialbot]

Submitting author: @armcdona (Ashley Ringer McDonald)
Repository: https://github.com/calpolyccg/MDSAPT
Branch with paper.md (empty if default branch): JOSS_Pub
Version: v2.0
Editor: @arfon
Reviewers: @wiederm, @CarvFS
Archive: Pending

Status

Status badge code:

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

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

@wiederm & @CarvFS, 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 @arfon 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 @CarvFS

📝 Checklist for @wiederm

[editorialbot]

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]

Software report:

github.com/AlDanial/cloc v 1.88  T=0.12 s (539.1 files/s, 60084.7 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          21            876           1707           2433
XML                              1              0              0            463
YAML                            14             59            120            380
Markdown                        10             99              1            297
TeX                              1             13              0            201
reStructuredText                 8             57             75             57
Bourne Shell                     3              8             10             43
JSON                             2              5              0             42
DOS Batch                        1              8              1             27
make                             1              4              6             10
Dockerfile                       1              4             12              3
-------------------------------------------------------------------------------
SUM:                            63           1133           1932           3956
-------------------------------------------------------------------------------


gitinspector failed to run statistical information for the repository

[editorialbot]

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

OK DOIs

- 10.1021/acs.jcim.8b00989 is OK
- 10.1093/bioinformatics/btx789 is OK
- 10.1063/5.0006002 is OK
- 10.1002/jcc.21787 is OK
- 10.25080/Majora-629e541a-00e is OK
- 10.1002/jcc.23753 is OK
- 10.1002/wcms.1452 is OK
- 10.6084/M9.FIGSHARE.17156018.V1 is OK
- 10.1002/wcms.84 is OK
- 10.1021/bk-2007-0958 is OK
- 10.1002/9780470399545.ch1 is OK
- 10.1016/j.bpj.2015.08.015 is OK
- 10.6084/m9.figshare.20520726.v1 is OK

MISSING DOIs

- None

INVALID DOIs

- None

[editorialbot]

Wordcount for paper.md is 1255

[editorialbot]

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

[arfon]

@wiederm, @CarvFS – This is the review thread for the paper. All of our communications will happen here from now on.

Please read the "Reviewer instructions & questions" in the first comment above. Please create your checklist typing:

@editorialbot generate my checklist

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.

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, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention https://github.com/openjournals/joss-reviews/issues/5931 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 the review process to be completed within about 4-6 weeks but please make a start well ahead of this as JOSS reviews are by their nature iterative and any early feedback you may be able to provide to the author will be very helpful in meeting this schedule.

[CarvFS]

Review checklist for @CarvFS

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 https://github.com/calpolyccg/MDSAPT?
• 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 (@armcdona) 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?

[CarvFS]

Review: observations on the paper and functionality

First of all: thank you very much for developing this toolkit! It is important to make the non-convalent bond analysis easier for everyonw working with molecular dynamics. Here I will post some comments on the paper and toolkit. Any new observations I will post as a new comment.

About installation:

I think the authors should update the documentation to avoid users running into issues while trying to install their toolkit. A few things I have observed:

• Python version 3.7 or older gives the error: "cannot import name 'Literal' from 'typing'";

• Trying to install as conda install -c psi4/label/dev -c conda-forge mdsapt is not working, it runs as

Retrieving notices: ...working... done
Collecting package metadata (current_repodata.json): done
Solving environment: unsuccessful initial attempt using frozen solve. Retrying with flexible solve.
Solving environment: unsuccessful attempt using repodata from current_repodata.json, retrying with next repodata source.
Collecting package metadata (repodata.json): done
Solving environment: / Killed

• Trying to install by cloning the repository and running pip install ./MDSAPT also did not worked:
  • I had to install all necessary tools (e.g pydantic and MDAnalysis) manually. I believe those packages and its versions should be listed in the documentation;
  • Them, I have got an error due to pydantic deprecating root_validator. I have openned an issue about this error.

Maybe using some specific version of pydantic would avoid my last error. However, it should be listed in the documentation which versions the user must have.

About important links

• Link to readthedocs page at the paper is not redirecting to the correct page;

• The binder demo are returning some error messages. I liked very much that you have made it, so new users could just run it first without need to install anything. It would be very nice to keep it updated.

About the paper and toolkit

• When it says "including" in line 35 of page 1, it gives the ideia that there are other tasks not described. Is this the case? Maybe describe the all workflow tasks as bullet points in the paper and/or add it to the documentation;

• It is said that a number of python based analysis tools exist for MD data. However, it is not clear if any of those also performs the same tasks implemented in MD-SAPT. For example: psi4 is used to run the quantum calculations, MDAnalysis is used to handle the MD simulation data. However, is not said running SAPT for each frame of a MD simulation is only possible with MD-SAPT toolkit;

• Psi4 has a wide range of options for running SAPT calculations (https://psicode.org/psi4manual/master/sapt.html#sapt0): for example, the default level of theory is SAPT0, but we also can run using SAPT2, SAPT2+ or SAPT2+3. Is it possible to change those options through MD-SAPT? If so, maybe the preformatted file should contain all possible settings with the standart values (since it is said on the paper that "This provides users with a preformatted file, such that they only need to fill in their parameters without any additional formatting").

• The example given in the paper is about the interactions between residuals in the Adenylate kinase. However, it seems that those kind of computations should be carried out using fisapt module (see https://psicode.org/psi4manual/master/fisapt.html#i-sapt-a-representative-example). It uses sapt0 as underlying methodology, but extends its capability. Having this in mind: how MD-SAPT prepares the input for running Psi4? Is it already using fisapt? If not, how the user should change this?

• In the line 29 of page 1, it is said: "offering detailed insight into the strength and type of noncovalent interactions interactions". It should be discussed why this understanding is important and how it could be applied for a non-specialist audience. Maybe also cite references https://doi.org/10.1063/5.0127106 and https://doi.org/10.1021/acs.jcim.2c00788;

• In line "This result was generated as a demonstration of the kinds of data MD-SAPT can generate". This phrasing can lead the reader to think there are other types of data which can be generated by MD-SATP. If it is the case, the authors should explain all the outputs that can be acquired.

• Is there any plans to add support to other topology formats, e.g. amber topology formats (.prmtop)?

  • It seems one can use ParmEd to convert from .prmtop to .psf (Transfer Amber topology and coordinate files to CHARMM or GROMACS formats ParmEd/ParmEd#631)... I will try this as soon as I get MD-SAPT running at my end, but it would be nice to mention that the user can convert to get the needed file format.

General comments

• The last commit to the MD-SAPT repository was made Oct 15, 2022. Is there any updates made after this?

[arfon]

👋 @wiederm – just checking in here, when do you think you might be able to complete your review?

[wiederm]

Hi @arfon , sorry for the delay! I will add my review on Monday!

[wiederm]

Review checklist for @wiederm

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 https://github.com/calpolyccg/MDSAPT?
• 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 (@armcdona) 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?

[wiederm]

Review

Thank you for providing this software.

Software

• Can you please re-enable the GH action tests to make sure that tests are passing?
• Unfortunately, I am having trouble installing the package with python=3.11. I added this to the issue: Problem with installation calpolyccg/MDSAPT#43 (comment)
• I'd love to see more documentation. Unfortunately, documentation is currently limited to the example provided. Please provide more details on the various use cases of this analysis, the options to control the package, and the cli. Also, as far as I can tell, the documentation is only accessible by going through the docs badge. Please provide a link in the README.

Manuscript

l40: comment needs to be removed
l53: comment needs to be resolved

• I'd love to hear more about the theory behind SAPT, why it is helpful to re-analyze MD simulations with different energy functions, and what kind of scientific questions can be answered with this toolkit. Please provide an example so that it is easier for the reader to understand the tool's purpose.
• Can you please describe (and document in the documentation) each option shown in Fig 1?
• Can you provide a guideline for choosing an appropriate QM level of theory?

[CarvFS]

Additional review:

Generating input file:

• Generating the input file as described creates a file with the an empty system_limits field

system_limits:
  ncpus: 
  memory: 

and this cause an error. For example, by running:

mdsapt generate test.yaml

and then,

import mdsapt

config = mdsapt.load_from_yaml_file('test.yaml')

I have got the error:

2023-11-21 12:00:58,888 mdsapt.config ERROR    Error while loading config from 'test.yaml'
Traceback (most recent call last):
  File "/home/felipe/miniconda3/envs/mdsapt/lib/python3.9/site-packages/mdsapt/config.py", line 309, in load_from_yaml_file
    return Config.model_validate(yaml.safe_load(file))
  File "/home/felipe/miniconda3/envs/mdsapt/lib/python3.9/site-packages/pydantic/main.py", line 503, in model_validate
    return cls.__pydantic_validator__.validate_python(
pydantic_core._pydantic_core.ValidationError: 2 validation errors for Config
system_limits.ncpus
  Input should be a valid integer [type=int_type, input_value=None, input_type=NoneType]
    For further information visit https://errors.pydantic.dev/2.5/v/int_type
system_limits.memory
  Input should be a valid string [type=string_type, input_value=None, input_type=NoneType]
    For further information visit https://errors.pydantic.dev/2.5/v/string_type
Traceback (most recent call last):
  File "/mnt/d/OneDrive - Universidade Federal de Minas Gerais/Pós doc/paper-review/MD-SAPT/testing/run_mdsapt.py", line 3, in <module>
    config = mdsapt.load_from_yaml_file('test.yaml')
  File "/home/felipe/miniconda3/envs/mdsapt/lib/python3.9/site-packages/mdsapt/config.py", line 312, in load_from_yaml_file
    raise err
  File "/home/felipe/miniconda3/envs/mdsapt/lib/python3.9/site-packages/mdsapt/config.py", line 309, in load_from_yaml_file
    return Config.model_validate(yaml.safe_load(file))
  File "/home/felipe/miniconda3/envs/mdsapt/lib/python3.9/site-packages/pydantic/main.py", line 503, in model_validate
    return cls.__pydantic_validator__.validate_python(
pydantic_core._pydantic_core.ValidationError: 2 validation errors for Config
system_limits.ncpus
  Input should be a valid integer [type=int_type, input_value=None, input_type=NoneType]
    For further information visit https://errors.pydantic.dev/2.5/v/int_type
system_limits.memory
  Input should be a valid string [type=string_type, input_value=None, input_type=NoneType]
    For further information visit https://errors.pydantic.dev/2.5/v/string_type

By filling ncpus and memory it ran without any issue. Since those parameters will vary from one user to another, it could be filled with a general case (ncpus = 1 and memory = '1Gb', for example) or explicit tell the user to fill those.

The way it is described in the paper, the reader will be lead to think that one just need to run mdsapt generate [filename] and not add/modify anything else before running the calculations.

• The generate command must creates a file for the user. Thus, maybe it would be a good idea to add some required flags for pairs, topology and trajectory, since those are very specific options.

Running SAPT calculations

• sapt_run.run(config.start, config.stop, config.step) throws an error:

 raise AttributeError(f'{type(self).__name__!r} object has no attribute {item!r}')
AttributeError: 'Config' object has no attribute 'start'

To make it run I have changed to

sapt_run.run(config.analysis.frames.start, config.analysis.frames.stop, config.analysis.frames.step)

This should be fixed in the paper example.

[arfon]

@armcdona – looks like there's a fair amount of feedback to start working on from the reviewers here. Could you get started and let us know here how you're getting on?

[Kevin-Mattheus-Moerman]

@armcdona 👋 we hope you are doing well. Could you please provide an update to the editor here? Thanks!

[ifd3f]

I'm currently working on packaging it with conda-forge (conda-forge/staged-recipes#24782). We were previously packaged with psi4's channel but since they're distributing with conda-forge now, we're moving our package there too. I've been very busy with work recently, and had to deal with a cold and strep, but I will get back to packaging this soon.

[arfon]

👋 @ifd3f – just checking in again here to see how you're getting along?

[ifd3f]

I got builds to work, now I need to make it pass the conda-forge CI. Hopefully I should have it published on conda-forge by end of week. Sorry about the wait.

[Kevin-Mattheus-Moerman]

@ifd3f thanks for that update. Were you able to get that done? Let us know if we can proceed. It would be good to proceed, to avoid loosing track of the reviewers.

[ifd3f]

It was finally merged into conda-forge after a lengthy process. I'll update the instructions in the paper once I get a chance.

[ifd3f]

Hi @arfon @Kevin-Mattheus-Moerman, sorry for the late update, but we are ready to proceed again.