openjournals / joss-reviews

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

[REVIEW]: alchemlyb: the simple alchemistry library #6934

Closed editorialbot closed 1 month ago

editorialbot commented 4 months ago

Submitting author: !--author-handle-->@orbeckst<!--end-author-handle-- (Oliver Beckstein) Repository: https://github.com/alchemistry/alchemlyb Branch with paper.md (empty if default branch): 71-joss-paper Version: 2.4.1 Editor: !--editor-->@srmnitc<!--end-editor-- Reviewers: @glycodynamics, @ryankzhu Archive: 10.5281/zenodo.13799342

Status

status

Status badge code:

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

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

@glycodynamics & @mikemhenry & @philbiggin, 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 @srmnitc 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 @glycodynamics

📝 Checklist for @ryankzhu

editorialbot commented 4 months 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 4 months ago

Software report:

github.com/AlDanial/cloc v 1.90  T=0.09 s (1054.2 files/s, 172073.0 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          50           2311           3704           6697
reStructuredText                34            502            370           1087
TeX                              1             40              0            280
Markdown                         1             41              0            186
YAML                             7             36             13            183
DOS Batch                        1              8              1             27
make                             1              4              6             10
-------------------------------------------------------------------------------
SUM:                            95           2942           4094           8470
-------------------------------------------------------------------------------

Commit count by author:

   139  Oliver Beckstein
    96  David Dotson
    82  Zhiyi Wu
    24  harlor
    12  Domenico Marson
     9  Ian Kenney
     7  Hyungro Lee
     6  trje3733
     5  shuai
     4  Jérôme Hénin
     4  Pascal Merz
     4  Victoria Lim
     3  Michael Shirts
     2  Irfan Alibay
     2  Mohammad Soroush Barhaghi
     2  Shuai Liu
     2  Tom Joseph
     2  hl2500
     1  Alexander Schlaich
     1  Bryce Allen
     1  David Mobley
     1  Wei-Tse Hsu
     1  brycestx
     1  helmut carter
editorialbot commented 4 months ago

Paper file info:

📄 Wordcount for paper.md is 2661

✅ The paper includes a Statement of need section

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

OK DOIs

- 10.1016/0021-9991(76)90078-4 is OK
- 10.1002/jcc.20290 is OK
- 10.1021/ct0502864 is OK
- 10.1016/j.softx.2020.100627 is OK
- 10.1021/jp807701h is OK
- 10.1021/acs.jcim.2c01052 is OK
- 10.21105/joss.01831 is OK
- 10.1063/1.1749657 is OK
- 10.1007/s10822-015-9840-9 is OK
- 10.1021/ct2003995 is OK
- 10.1063/1.3607597 is OK
- 10.1063/5.0014475 is OK
- 10.1021/jp102971x is OK
- 10.1016/j.softx.2015.06.001 is OK
- 10.1063/1.2978177 is OK
- 10.1063/1.1638996 is OK
- 10.1063/1.1740409 is OK
- 10.1007/s10822-019-00267-z is OK
- 10.33011/livecoms.2.1.18378 is OK
- 10.1021/acs.jctc.5b00784 is OK
- 10.25080/Majora-92bf1922-00a is OK
- 10.1016/j.cpc.2021.108171 is OK
- 10.1021/acs.jctc.8b00447 is OK
- 10.33011/livecoms.5.1.2067 is OK

MISSING DOIs

- No DOI given, and none found for title: Drug design: structure-and ligand-based approaches
- No DOI given, and none found for title: Scikit-learn: Machine Learning in Python
- No DOI given, and none found for title: API design for machine learning software: experien...
- No DOI given, and none found for title: Simulation techniques for solvation-induced surfac...

INVALID DOIs

- None
editorialbot commented 4 months ago

License info:

✅ License found: BSD 3-Clause "New" or "Revised" License (Valid open source OSI approved license)

editorialbot commented 4 months ago

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

srmnitc commented 4 months ago

👋🏼 @orbeckst @glycodynamics, @mikemhenry, @philbiggin this is the review thread for the paper. All of our communications will happen here from now on.

As a reviewer, the first step is to create a checklist for your review by entering

@editorialbot generate my checklist

as the top of a new comment in this thread.

These checklists contain the JOSS requirements. As you go over the submission, please check any items that you feel have been satisfied. The first comment in this thread also contains 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 just judging this submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention openjournals/joss-reviews#REVIEW_NUMBER 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 any of you require some more time. We can also use EditorialBot (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 (@srmnitc ) if you have any questions/concerns, thanks again for the submission, and for the reviews

glycodynamics commented 4 months ago

Review checklist for @glycodynamics

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

glycodynamics commented 4 months ago

@srmnitc is there a due date?

srmnitc commented 4 months ago

It would be ideal to complete the review within about 2-4 weeks. If you need more time, please let me know.

@srmnitc is there a due date?

mikemhenry commented 4 months ago

@srmnitc So sorry but I will have to withdraw as a reviewer, I didn't look at the author list closely when I agreed to review (my bad!) and I have a COI from publishing with one of the authors in the last 4 years

philbiggin commented 4 months ago

If this is an issue (ie publishing with one of the authors in the last four years) then I would also have to withdrawn Best wishes, Phil


Prof Phil Biggin T: 01865 613305 W: sbcb.bioch.ox.ac.uk/biggin.php E: @.***

On 27 Jun 2024, at 21:09, Mike Henry @.***> wrote:



@srmnitchttps://github.com/srmnitc So sorry but I will have to withdraw as a reviewer, I didn't look at the author list closely when I agreed to review (my bad!) and I have a COI from publishing with one of the authors in the last 4 years

— Reply to this email directly, view it on GitHubhttps://github.com/openjournals/joss-reviews/issues/6934#issuecomment-2195586579, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AMMKVGQUDWR5C2XR23UIMSDZJRWODAVCNFSM6AAAAABJ7KO7RGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJVGU4DMNJXHE. You are receiving this because you were mentioned.Message ID: @.***>

mikemhenry commented 4 months ago

@philbiggin This is the policy https://joss.readthedocs.io/en/latest/reviewer_guidelines.html#joss-conflict-of-interest-policy fyi

philbiggin commented 4 months ago

In that case I am definitely conflicted as having published with several of the authors.

Phil


Prof Phil Biggin Professor of Computational Biochemistry Chair of the Molecular Graphics and Modelling Society Fellow and Tutor at Lady Margaret Hall, Oxford Department of Biochemistry South Parks Road Oxford OX1 3QU E: @.*** W: sbcb.bioch.ox.ac.uk/biggin.php T: +44 1865 613305

On 28 Jun 2024, at 01:14, Mike Henry @.***> wrote:



@philbigginhttps://github.com/philbiggin This is the policy https://joss.readthedocs.io/en/latest/reviewer_guidelines.html#joss-conflict-of-interest-policy fyi

— Reply to this email directly, view it on GitHubhttps://github.com/openjournals/joss-reviews/issues/6934#issuecomment-2195862336, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AMMKVGW2FJJKN75DHBMIHLTZJSTHRAVCNFSM6AAAAABJ7KO7RGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJVHA3DEMZTGY. You are receiving this because you were mentioned.Message ID: @.***>

srmnitc commented 4 months ago

@mikemhenry and @philbiggin thank you for bringing this up. I will unassign you as reviewers. Thanks for your efforts nevertheless!

srmnitc commented 4 months ago

@editorialbot remove @mikemhenry from reviewers

editorialbot commented 4 months ago

@mikemhenry removed from the reviewers list!

srmnitc commented 4 months ago

@editorialbot remove @philbiggin from reviewers

editorialbot commented 4 months ago

@philbiggin removed from the reviewers list!

srmnitc commented 4 months ago

@editorialbot add @ryankzhu as reviewer

editorialbot commented 4 months ago

@ryankzhu added to the reviewers list!

srmnitc commented 4 months ago

@ryankzhu thanks for agreeing to be reviewer. You can start you review by using @editorialbot generate my checklist. Here, you can find some information about the checklist. Of course, feel fre e to ping/email me if you need any help. We aim to finish the review in 2-4 weeks. If you need more time, please let me know.

ryankzhu commented 3 months ago

Review checklist for @ryankzhu

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

srmnitc commented 3 months ago

@glycodynamics @ryankzhu Thanks again for agreeing to review. Just a short reminder from my side. If you need any help from me, please let me know.

glycodynamics commented 3 months ago

@srmnitc I am sorry for the delay. I will write comments by the coming Tuesday.

glycodynamics commented 3 months ago

@srmnitc Should I post my comments directly here?

srmnitc commented 3 months ago

@glycodynamics The best option is to open separate issues directly on the code repository for each issue that you find. In this issue, it would be great to also add the link of this review issue so that they show up here. This way, its easier to keep track. If they are small issues, you could also simply write here.

glycodynamics commented 3 months ago

@srmnitc It is not related to code. The code worked perfectly fine for the test case I tried. My comments are related to the "State of the field" and "Example usage" sections, which I did not check off in my checklist.

For editor only: I noticed some co-authors contributed less code and commits than one who has been just acknowledged. Also, at least one co-author is not listed in the copyright section of the code but is listed as a co-author in the paper. I understand that the contributions of each author are best known to the corresponding author and not raising a flag but pointing it out as there was a "Contribution and authorship" section in the checklist.

ryankzhu commented 3 months ago

@srmnitc I am sorry for the delay. The codes have worked fine in my test case. The documentation is quite detailed and the manuscript reads nicely and concisely to me. I only have two small comments:

orbeckst commented 3 months ago

Thank you for your reviews @glycodynamics and @ryankzhu .

Would you be able to open issues in https://github.com/alchemistry/alchemlyb/issues for

We will address @glycodynamics 's comments in the paper

cc first author @xiki-tempula

orbeckst commented 3 months ago

I noticed some co-authors contributed less code and commits than one who has been just acknowledged. Also, at least one co-author is not listed in the copyright section of the code but is listed as a co-author in the paper. I understand that the contributions of each author are best known to the corresponding author and not raising a flag but pointing it out as there was a "Contribution and authorship" section in the checklist.

The PR https://github.com/alchemistry/alchemlyb/pull/328 contains the full history of who decided to be an author and full rationale for all decisions regarding authorship. I am happy to explain in more detail.

glycodynamics commented 3 months ago

We will address @glycodynamics 's comments in the paper

  • Discussion of similar tools would be helpful for context and it's missing in the manuscript.

    • We do mention alchemical-analysis.py ; I am not aware of other tools that do the same as alchemlyb in terms of being applicable to many different file format, but we'll look carefully. If you have any specific software in mind please let us know!
  • I noticed that the description of errors is missing from the paper, which could be a valuable addition.

    • Do you mean estimation of errors?

cc first author @xiki-tempula

@orbeckst

Thanks and best wishes.

xiki-tempula commented 3 months ago

@glycodynamics Thanks for the review. I'm glad that you mentioned the free energy analysis tool in SOMD. So analyse_freenrg from siremol is deprecated and their more recent tool for doing free energy analysis is based on alchemlyb https://github.com/OpenBioSim/biosimspace/blob/devel/python/BioSimSpace/FreeEnergy/_relative.py#L1241.

I meant to say estimation of errors, how are they obtained, internally in alchemylib or coming from the external program, such as bootstrapping in MBAR? I may have missed, but I didn't find good documentation of this.

We just use the error estimation function from pymbar, so it might not be too useful to say in the paper that we just use pymbar for error estimation.

srmnitc commented 3 months ago

@editorialbot generate pdf

editorialbot commented 3 months ago

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

srmnitc commented 3 months ago

@glycodynamics and @ryankzhu thanks for the review, and thanks @orbeckst and @xiki-tempula for agreeing to address the issues.

@glycodynamics Thanks again for looking closely at the author list. @orbeckst going through https://github.com/alchemistry/alchemlyb/pull/328 it looks like the author list is justified. However, as @glycodynamics pointed out there are differences between copyright statement on the webpage and the author list. Is there a reason for this difference? Would it make more sense to be consistent?

orbeckst commented 3 months ago

We should be consistent between AUTHORS and doc footer and I raised an issue to address this discrepancy.

orbeckst commented 3 months ago

We fixed the inconsistent author display in the docs by linking to the authoritative AUTHORS file. Please see cross-linked issue alchemistry/alchemlyb#383 .

srmnitc commented 3 months ago

@glycodynamics could you please take a look again when possible and see if your issues have been addressed?

glycodynamics commented 3 months ago

@orbeckst Thanks so much for the revision.

@srmnitc I read both the tutorial and how authors are now handling the list of developers. I agree that mentioning about the error estimation function is not required unless it was handled somehow differently within the alchemlyb. I am happy with the revision and I do not have any further comments.

srmnitc commented 3 months ago

@orbeckst Thanks so much for the revision.

@srmnitc I read both the tutorial and how authors are now handling the list of developers. I agree that mentioning about the error estimation function is not required unless it was handled somehow differently within the alchemlyb. I am happy with the revision and I do not have any further comments.

Thanks @glycodynamics. In that case, could you please review your checklist again, and confirm that you recommend this paper for publication? Thanks a lot once again for your review!

srmnitc commented 3 months ago

@ryankzhu Thanks for your review again. Could you please confirm that you recommend this paper for publication?

glycodynamics commented 3 months ago

@orbeckst Thanks so much for the revision. @srmnitc I read both the tutorial and how authors are now handling the list of developers. I agree that mentioning about the error estimation function is not required unless it was handled somehow differently within the alchemlyb. I am happy with the revision and I do not have any further comments.

Thanks @glycodynamics. In that case, could you please review your checklist again, and confirm that you recommend this paper for publication? Thanks a lot once again for your review!

@srmnitc I recommend this manuscript for publication.

srmnitc commented 2 months ago

@orbeckst I have asked @ryankzhu also by email to confirm that they recommend the publication of this software. After that, I will go ahead with the rest of the steps. Thanks a lot for your patience!

ryankzhu commented 2 months ago

@srmnitc Sorry for the late reply. I confirm I recommend this manuscript for publication.

srmnitc commented 2 months ago

@glycodynamics and @ryankzhu thanks a lot for taking your time for the reviews!

srmnitc commented 2 months ago

@editorialbot generate pdf

editorialbot commented 2 months ago

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

srmnitc commented 2 months ago

@editorialbot check references

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

OK DOIs

- 10.1016/0021-9991(76)90078-4 is OK
- 10.1002/jcc.20290 is OK
- 10.1021/ct0502864 is OK
- 10.1016/j.softx.2020.100627 is OK
- 10.1021/jp807701h is OK
- 10.1021/acs.jcim.2c01052 is OK
- 10.21105/joss.01831 is OK
- 10.1063/1.1749657 is OK
- 10.1007/s10822-015-9840-9 is OK
- 10.1021/ct2003995 is OK
- 10.1063/1.3607597 is OK
- 10.1063/5.0014475 is OK
- 10.1021/jp102971x is OK
- 10.1016/j.softx.2015.06.001 is OK
- 10.1063/1.2978177 is OK
- 10.1063/1.1638996 is OK
- 10.1063/1.1740409 is OK
- 10.1007/s10822-019-00267-z is OK
- 10.33011/livecoms.2.1.18378 is OK
- 10.1021/acs.jctc.5b00784 is OK
- 10.25080/Majora-92bf1922-00a is OK
- 10.1016/j.cpc.2021.108171 is OK
- 10.1021/acs.jctc.8b00447 is OK
- 10.33011/livecoms.5.1.2067 is OK

MISSING DOIs

- No DOI given, and none found for title: Drug design: structure-and ligand-based approaches
- No DOI given, and none found for title: Scikit-learn: Machine Learning in Python
- No DOI given, and none found for title: API design for machine learning software: experien...
- No DOI given, and none found for title: Simulation techniques for solvation-induced surfac...

INVALID DOIs

- None