search

openjournals / joss-reviews

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

[REVIEW]: matscipy: materials science at the atomic scale with Python #5668

Closed editorialbot closed 10 months ago

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@pastewka<!--end-author-handle-- (Lars Pastewka) Repository: https://github.com/libAtoms/matscipy Branch with paper.md (empty if default branch): Version: v1.0.0 Editor: !--editor-->@diehlpk<!--end-editor-- Reviewers: @rashatwi, @mbarzegary, @mastricker Archive: 10.5281/zenodo.10564956

Status

status

Status badge code:

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

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

@rashatwi, 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 @diehlpk 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 @mbarzegary

📝 Checklist for @rashatwi

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=2.07 s (141.2 files/s, 217296.0 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
F#                               2              1              0         318016
HTML                             4           9399              8          59776
Python                         189           9112          15522          27517
TeX                              1             67              6           1110
SVG                              1              0             12           1014
C++                              4            301            269            942
C                                3            160            159            751
Jupyter Notebook                10              0           3263            650
Markdown                         5            100              0            322
Bourne Shell                    28             29             30            259
Meson                           19             46             56            246
YAML                             5             45             29            230
C/C++ Header                     7             98            141            222
DOS Batch                        1             29              1            212
make                             1             29              6            143
reStructuredText                 8             70             68            107
Ruby                             1             28             12            106
XML                              2             12              6             78
TOML                             1              3              0             40
INI                              1              0              0              2
-------------------------------------------------------------------------------
SUM:                           293          19529          19588         411743
-------------------------------------------------------------------------------

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

Wordcount for paper.md is 2535

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.1088/0965-0393/18/1/015012 is OK
- 10.1088/0034-4885/72/2/026501 is OK
- 10.1021/acs.jctc.8b00959 is OK
- 10.1063/1.5035508 is OK
- 10.1039/d0cp01841d is OK
- 10.1103/PhysRevMaterials.4.023601 is OK
- 10.1016/j.actamat.2023.118734 is OK
- 10.1007/s11249-009-9566-8 is OK
- 10.1103/PhysRevB.31.5262 is OK
- 10.1016/j.commatsci.2006.07.013 is OK
- 10.1103/PhysRevLett.56.632 is OK
- 10.1103/PhysRevB.39.5566 is OK
- 10.1103/PhysRevLett.64.1955 is OK
- 10.1088/0959-5309/43/5/301 is OK
- 10.1007/BF00186854 is OK
- 10.1080/14786437508226544 is OK
- 10.1103/PhysRevLett.115.135501 is OK
- 10.1103/PhysRevE.103.033002 is OK
- 10.1002/jcc.21224 is OK
- 10.1007/978-3-7091-8752-4 is OK
- 10.1007/s11249-020-01395-6 is OK
- 10.1137/040609938 is OK
- 10.1021/ja9621760 is OK
- 10.1016/j.cpc.2021.108171 is OK
- 10.1021/jacs.5b04073 is OK
- 10.1103/PhysRevLett.124.105501 is OK
- 10.1021/acsami.9b18019 is OK
- 10.1007/s11249-021-01508-9 is OK
- 10.3390/ma15093247 is OK
- 10.1007/978-3-642-23099-8 is OK
- 10.1103/PhysRevMaterials.7.055601 is OK
- 10.1103/PhysRevMaterials.7.073603 is OK
- 10.1088/0965-0393/17/5/053001 is OK
- 10.1016/j.jmps.2018.05.004 is OK
- 10.1103/PhysRevB.74.075420 is OK
- 10.1103/PhysRevB.86.075459 is OK
- 10.1103/PhysRevE.57.7192 is OK
- 10.1088/2515-7639/ab36ed is OK
- 10.1557/mrc.2019.93 is OK
- 10.1103/PhysRevMaterials.4.013603 is OK
- 10.1103/PhysRevB.44.4925 is OK
- 10.1103/PhysRevB.78.161402 is OK
- 10.1088/1361-651X/ab45da is OK
- 10.1080/23746149.2022.2093129 is OK
- 10.48550/arXiv.2205.06643 is OK
- 10.1016/j.carbon.2015.10.098 is OK
- 10.1103/PhysRevMaterials.2.083601 is OK
- 10.1103/PhysRevLett.127.126101 is OK
- 10.1007/s11249-011-9864-9 is OK
- 10.1038/nmat2902 is OK
- 10.1007/s11249-020-01395-6 is OK
- 10.1063/5.0153397 is OK
- 10.1038/s41586-020-2649-2 is OK
- 10.1038/s41592-019-0686-2 is OK
- 10.1016/j.cpc.2015.07.012 is OK
- 10.1103/PhysRevB.29.6443 is OK

MISSING DOIs

- None

INVALID DOIs

- None
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:

diehlpk commented 1 year ago

Hi @rashatwi how is your review going?

rashatwi commented 1 year ago

Sorry for my delayed response. I'll complete the review this upcoming week. Thanks for your patience.

mastricker commented 1 year ago

@editorialbot generate my checklist

mastricker commented 1 year ago

** General checks

** Functionality

** Documentation

** Software paper

Summarizing: I would like to see the tests on my machine pass, other than that I am very happy with this contribution. Will definitely incorporate this into my own work.

As for the documentation issue: Maybe the authors have examples for more usage that are not easily findable for me and they could point me to the right example/folder/whatever to have a look at them.

diehlpk commented 1 year ago

Hi @pastewka please take a look at the comments above.

pastewka commented 1 year ago

Will work on those, thanks @diehlpk and @mastricker for the comments.

mbarzegary commented 1 year ago

Hi @diehlpk and @pastewka I confirm the quality of the software and the effort the authors have put in to develop it. The software contains lots of useful functionalities for bridging MD and QM simulations to the continuum scale, a challenge that most multi-scale modeling workflow and pipelines deal with. I think it will have a great impact on the target community, and I will use it myself for relevant projects for sure. However, I believe some minor concerns should be addressed before accepting the submission, elaborated in a couple of issues I have opened on the software repo (listed above). I hope the authors find the comments useful.

mbarzegary commented 1 year ago

Review checklist for @mbarzegary

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

pgrigorev commented 1 year ago

Dear @mastricker, thank you very much for your review and comments. Here I will try to answer your questions about dislocation module and the related tests.

To generate dislocations the module relies on anisotropic elasticity solution within Stroh formalism as implemented in atomman package. Unfortunately this package has an insane number of dependencies and thus we prefer not to include it as a default dependency of matscipy. This is one of the reasons so many tests test_dislocation.py are skipped (marked as s). We have been working on our own implementation of Stroh solution in order to have matscipy more independent. This is still in progress and requires extensive testing before the change.

Another reason why most of test are skipped is related to the use of DXA algorithm for testing. We simply tests if the DXA algorithm from ovito package detects the same dislocation that we want to create. This requires ovito python interface and thus is skipped in the automated testing if ovito python module not found. Again, we want to avoid including ovito as a standard dependency since we only use it for testing.

Finally, some of the tests rely on calling LAMMPS via LAMMPSlib ASE interface since it is one of the most common calculators for MD simulations. The test use an eam potential for tungsten to run a couple of basic checks. This tests are skipped if lammps python is not found. I think in your case you have lammps.py installed, but it might not have manybody package installed. This package is required to use eam potentials and is not a part of standard installation. This is the reason you have test_elastic_constants_lammpslib and test_gamma_line tests failing.

Normally I use these tests to verify a fresh installation of matscipy on a new machine in order to run calculations with LAMMPS via ASE. In this case I would have matscipy + atomman + lammps installed. I use the tests requiring ovito DXA when adding new dislocation configurations during development. It is true that this is not explained at all in the documentation and I will add a installation section to plasticity documentation with the detailed explanation.

In the plasticity section of the documentation I have added few examples how to build dislocations in Building cylindrical configurations with dislocations section. At the moment it is only for the case of BCC metals, I will add FCC and Diamond structure this week. I was thinking to add a section to show how one can add a new types of structures and dislocations to the set. If you have any particularly interesting system in mind, I would be happy to try to use it for this example.

rashatwi commented 1 year ago

Review checklist for @rashatwi

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

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:

rashatwi commented 1 year ago

@diehlpk and @pastewka:

I finished my review of the code and paper. It is clear to me that matscipy is well suited for JOSS and will be very useful to the computational materials science community.

In terms of the paper, I think the authors did a good job in terms of describing clearly what the functionalities of matscipy are and in terms of citing the relevant and commonly used packages in the field.

In terms of the checklist items, I have checked off most of them, but I think there are a few things that need to be addressed before acceptance of this work in JOSS. Most of my comments are minor, except for the fact that there is very limited/not enough examples or tutorials on how to use the different functionalities explained in the paper. I think the availability of detailed and clear tutorials will make it more accessible and much easier for new users to adopt the methods implemented in matscipy.

Paper-related comments:

  1. Typo on line 155 (ot?)
  2. Wondering if there is a way to show some of the equations used in the analysis codes like the radial and angular correlation functions? Multiple variations exist for these calculations (e.g., different normalization methods) and it would be nice for the user to know how these are exactly calculated without having to go over the code (either in the paper or on the package website).
  3. In the All-purpose atomic analysis tools section of the paper, only the topology building for non-reactive MD simulations subsection includes the specific module where the functionality is implemented, the other 4 subsections do not refer to the module and I had to go over the package modules to know where these are implemented.

Package-related comments:

  1. I suggest adding the package sphinx documentation link in the “About” section at the top right of the GitHub repository. It makes it much more accessible than just referring to it in the README file.
  2. I see there is support for command line interface, but there is no details about how to use it. I’m not sure if this is a preliminary implementation of the cli. If not, then maybe adding simple examples in the README file will be helpful.
  3. In the matscipy.elasticity module, lines 1303 and 1304, spilu and LinearOperator are not properly imported from scipy.
  4. I created a separate issue for the examples folder provided in the package root directory.
diehlpk commented 1 year ago

@pastewka please have a look at the comments above.

mastricker commented 1 year ago

Dear @pgrigorev thank you very much for the explanation. I understand all your points from my own experience! Please consider my remarks resolved with the explanation and additional details.

Great work y'all. I'm looking forward to using the package in my own work.

pgrigorev commented 1 year ago

@mastricker thank you very much for your feedback. I am working on an update of the documentation of dislocation module to take into account your comments. If you have any problems with the package in the future please do not hesitate to open an issue and/or contact me directly.

diehlpk commented 1 year ago

@pgrigorev could you finish the requested changes?

diehlpk commented 1 year ago

Hi @mbarzegary how is your review going?

mbarzegary commented 1 year ago

hi @diehlpk I did the review 2 weeks ago and opened a set of issues on the software repo, listed above.

pgrigorev commented 1 year ago

@pgrigorev could you finish the requested changes?

I have just added the requested changes to a pull request. Beginning of next week we will make sure everything works and it should be merged to update the documentation.

pgrigorev commented 1 year ago

@pgrigorev could you finish the requested changes?

I have just added the requested changes to a pull request. Beginning of next week we will make sure everything works and it should be merged to update the documentation.

Hello @diehlpk and @mastricker. We have successfully updated the documentation by adding more details about tests and installation of matscipy.dislocation module as well as tutorials on how to create dislocations in BCC, FCC and Diamond structures. You can have a look here.

Edit: @thomas-rocke added a tutorial on multi-species systems to documentation and a short mention of this functionality in the manuscript.

diehlpk commented 1 year ago

@mastricker could you please have a look?

diehlpk commented 1 year ago

Hi @rashatwi how is your review going?

mastricker commented 1 year ago

@pgrigorev This is great!

@diehlpk : I'm good. Is there anything that I need to "officially" confirm? I have updated my comment above with a positive checkmark for the documentation.

diehlpk commented 12 months ago

@pgrigorev This is great!

@diehlpk : I'm good. Is there anything that I need to "officially" confirm? I have updated my comment above with a positive checkmark for the documentation.

@mastricker No you just need to check all boxes. Thanks for the review.

diehlpk commented 12 months ago

Hi @rashatwi how is your review going?

rashatwi commented 11 months ago

@editorialbot generate pdf

editorialbot commented 11 months ago

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

rashatwi commented 11 months ago

Hi @diehlpk, I did my review in October, my comments related to the paper were not addressed. I just updated my checklist by ticking the boxes related to the examples and documentation.

pastewka commented 11 months ago

Thanks everyone for the great work reviewing this. I am not really on top of things these days, but will look into this over the next weeks to address the remaining issues.

diehlpk commented 11 months ago

@pastewka Can we please try to finish the review before the Christmas break?

pastewka commented 11 months ago

@diehlpk I'll try, but I think this will not be possible on my end.

diehlpk commented 11 months ago

@pastewka ok, I will be traveling the first two weeks in the new year and will proceed with the paper mid January.

pastewka commented 10 months ago

@diehlpk - We are working on addressing remaining issue but should be finished by end of next week. I will post a summary of all reviewer comments and a response here when we are done.

pastewka commented 10 months ago

@editorialbot generate pdf

editorialbot commented 10 months ago

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

pastewka commented 10 months ago

@diehlpk - We finished addressing the reviewer comments, response letter follows here. I am also tagging my coauthors @jameskermode and @pgrigorev

We thank all three reviewers for the time devoted to looking at manuscript, documentation and code. This was extremely helpful to streamlining matscipy and we think that in particular the documentation of the code has tremendously improved in the process.

Below we have collected the reviewers comments from different locations (JOSS issue track, matscipy issue tracker) and briefly outline our actions in response to the comments. We hopefully didn't miss any comments as they were in different places - if we did, please accept our apologies and let us know where the additional comments are.

From @mastricker:

I have the impression that not all features are covered in the online documentation that is automatically generated by SPHINX; What would be of particular interest for my is a documentation how to run the DXA; or even to generate specific dislocation configurations in the first place. Part of the functionality is buried in the API reference section of the online documentation.

We cannot run DXA within matscipy but rely on the Ovito module for it. We are using DXA to test the dislocation generation. In the matscipy version during original submission those tests were actually deactivated in CI, we now activated them. Ovito and atomman are now installed as dependencies when installing matscipy with the [test] option.

Regarding dislocation-generation, we have substantially expanded the online documentation to include examples of it: https://libatoms.github.io/matscipy/applications/plasticity.html

_Presumably test_dislocation.py provides unclean output of testdislocation.py ssssssssssssss.Fssssssss.Fs.ssssssss.s

This was due to a lack of atomman and ovito packages. We have added both packages to the test dependencies in pyproject.toml, see also the above comment.

_Presumably test_energy_release.py provides additional s as output python -m pytest . results in a segmentation fault in testneighbours.py -> Fatal Python error: Segmentation fault

Unfortunately we cannot reproduce this failure. We will keep this in mind when continuing to develop matscipy. Note that our longer term agenda includes switching the C-bindings to pybind11, which should make the code more robust, in particular with regards to buffer overflows which may have been behind this segmentatio fault.

From @rashatwi:

Wondering if there is a way to show some of the equations used in the analysis codes like the radial and angular correlation functions? Multiple variations exist for these calculations (e.g., different normalization methods) and it would be nice for the user to know how these are exactly calculated without having to go over the code (either in the paper or on the package website).

We have now put the equation into the documentation and added a documentation page on the angle distribution. We think that those equations would be too much detail for the JOSS paper, but agree that they should be mentioned in the documentation.

In the All-purpose atomic analysis tools section of the paper, only the topology building for non-reactive MD simulations subsection includes the specific module where the functionality is implemented, the other 4 subsections do not refer to the module and I had to go over the package modules to know where these are implemented.

We removed the mention of explicit module names from the paper because we want to keep the option to refactor the code in the future without invalidating the paper. The package documentation should be the central source of information for how to actually run the code.

I suggest adding the package sphinx documentation link in the “About” section at the top right of the GitHub repository. It makes it much more accessible than just referring to it in the README file.

This is a good suggestion. We added this link to the about section.

I see there is support for command line interface, but there is no details about how to use it. I’m not sure if this is a preliminary implementation of the cli. If not, then maybe adding simple examples in the README file will be helpful.

It is correct that the code has a CLI that packages some workflows. There was also a subdirectory “scripts” that is not described anywhere. We have now moved some of the “scripts” to an actual (installable) CLI and added a section to the online documentation of the code on the CLI, and renamed “scripts” to “staging” to emphasize the transitory nature of the scripts stored there. Once matured, those scripts will move to the actual CLI, which is installed with matscipy and documented.

In the matscipy.elasticity module, lines 1303 and 1304, spilu and LinearOperator are not properly imported from scipy.

This was a good catch. We fixed this.

I see there is an examples folder in the root directory with descriptive names for the python scripts, but it is still not very clear to me what these examples do. I think it will be helpful to have a tutorials section on the website or Jupyter notebooks added to the main GitHub repository that guide the user step-by-step to how they can start using the functionalities described in the paper and also contain detailed explanations.

We thank the reviewer for this criticism, which we agree with. We have transitioned many of the examples to notebook tutorials included in the documentation pages (which has the further benefit that they are run in CI), and for the others have at the minimum added README files to all subdirectories. \

_On a side note, a lot of the examples provided in that directory have not been updated in a long time, e.g., in the example under fracture_mechanics/energy_barrier/params.py, I’m getting this import error: \ ImportError: cannot import name 'diamond_111_110' from 'matscipy.fracturemechanics.clusters’

We have removed the defunct examples and replaced them with tutorials which are included in the CI and run automatically to ensure they remain up to date. The examples which remain have had minimal README files added and over time they will be transitioned into full tutorial notebooks which can be included in the documentation and CI.

From @mbarzegary:

Some of the mentioned functionalities of the package are not documented. For example, there is no documentation for the electrochemistry. Also, some of the atomic analysis tools and all the interatomic potential features are only documented via autogenerated API docs. I think this makes it hard for beginner users to get started with the package. The application domains (except the electrochemistry, as mentioned) are well documented and described. It can be beneficial to talk a little bit about the package features in the documentation.

We are sorry for this, as there was indeed no documentation for the electrochemistry module. We now added this part of the documentation. We agree that not all code functionality is documented in the hand-written documentation, but we do think that the core is now documented. We will add documentation as we continue to develop the code.

It's mentioned that the package has an interface to the FEniCS code. This functionality is supported by one example that does not run. FEniCS dependency is not documented and elaborated.

This is now documented in the electrochemistry documentation.

I cannot find the FEniCS functionality being tested via the pytest tests. Is it because of the complicated installation procedure? I care about this functionality not only due to its usefulness but also because it's mentioned in the paper.

We now added FEniCS tests to CI. It will run automatically on a user installation if a FEniCS installation is detected. Note that (unlike Ovito and atomman) we have not added FEniCS to the [test] optional specialization of matscipy as FEniCS is not simply installable via pip.

All tests pass on my local machine, but some of them produce unclear characters. This is the output of the pytest run:

Some tests were probably skipped (resulting in an ‘s’ state) because of missing dependencies (Ovito, atomman, FEniCS). All tests that rely on any of these modules skip automatically if they are not present. Note that all tests run in CI.

_It's not clear where the user should start using the package after installing it. The way the provided examples are placed is a bit difficult to follow. Except for the electrochemistry and the fracturemechanics directories, the rest of the examples directories are not clear regarding which functionality they belong to. I suggest that you add a new example directory for getting started with the package, putting some basic examples grouped per feature.

We changed the entry page of the documentation and replaced it with a very brief getting started page.

It's not clear how the distributed computation example is supposed to work.

We have removed this module. It was a legacy module that was unmaintained.

Generally speaking, the example directory contains a large number of non-trivial files, most of which are not documented unfortunately. I think this makes it difficult to get started with the package.

This comment was also raised by rashatwi. We have added CLI documentation, moved some examples to the documentation and added README files to the examples. The longer term goal is to transition all examples to the documentation.

_Running the samples_pb_c2d.py and samples_pnp_c2d.py examples results in the following error: ModuleNotFoundError: No module named 'matscipy.electrochemistry.utility'. The error for samples_pnp_by_fenics_fem.py is related to lack of matscipy.electrochemistry.poisson_nernst_planck_solverfenics module.

We thank you for checking this, this problem has been fixed.

Some examples rely on IPython, which doesn't get installed via the installation procedure of the package.

We are not sure which examples the reviewer refers to. Note that we transitionen some examples to docs, and none of the remaining examples require IPython.

As mentioned in the paper, there exists other packages for multi-scale coupling such as LibMultiScale and MultiBench, but the necessity of the proposed package is not well elaborated. Since it’s the most important paragraph of the statement of need, I appreciate it if the authors elaborate on this.

We have rewritten this part of the paper to more explicitly mention other packages and their capabilities.

I see duplicate references. For example, there are two identical Seidel et al. (2021a and 2021b). Can you please check the paper for this?The authors have referred to 2 papers using the coupling of the numerical models of the Poisson–Nernst–Planck equation in electrochemistry (Hörmann et al., 2023; Seidl et al. 2021b), but it doesn't seem that those papers use this coupling. Can you please elaborate on the mentioned usage?

We have removed those citations. They were examples for specific atomic phenomena but the reviewer is right that matscipy is not used in those works and citing them may confuse the reader.

diehlpk commented 10 months ago

@mastricker please take a look at the author's response.

mastricker commented 10 months ago

@mastricker please take a look at the author's response.

I'm good - I did not know that I had to look again. In my earlier response I wrote that once these issues are addressed, you can proceed.

@pastewka @pgrigorev @jameskermode I applaud you for your thorough response and you software. I'm really looking forward to putting it to use very soon.

Cheers, M

pastewka commented 10 months ago

@diehlpk - our longer text above also contains responses to @rashatwi and @mbarzegary. Only @mbarzegary has still unchecked boxes in the report above.

diehlpk commented 10 months ago

Hi @mbarzegary could you please look at the changes and finish your review?

diehlpk commented 10 months ago

@mastricker please take a look at the author's response.

I'm good - I did not know that I had to look again. In my earlier response I wrote that once these issues are addressed, you can proceed.

@pastewka @pgrigorev @jameskermode I applaud you for your thorough response and you software. I'm really looking forward to putting it to use very soon.

Cheers, M

Thanks, your review list was not referenced in the issue, but I found it now.

diehlpk commented 10 months ago

@diehlpk - our longer text above also contains responses to @rashatwi and @mbarzegary. Only @mbarzegary has still unchecked boxes in the report above.

@pastewka Thanks, I reminded him.

mbarzegary commented 10 months ago

Thanks for the elaborated reply. @diehlpk I confirm that the authors have addressed my comments. I checked all the boxes and closed the issues I had opened on the software repo.

diehlpk commented 10 months ago

@pastewka I think we are done with the reviews :)

I will do some editorial checks and we are good to go for acceptance.

diehlpk commented 10 months ago

@editorialbot check references