Giter VIP home page Giter VIP logo

Comments (124)

rashatwi avatar rashatwi commented on August 20, 2024 1

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

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024 1
  • We have release v1.0.0
  • I've generate a Zenodo record but not yet released it. I would like to point the Zenodo record back to this paper once it has a DOI. The DOI for the Zenodo record is 10.5281/zenodo.10564956. If you can give me a DOI for the JOSS paper and a page number I can release the Zenodo record.

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024

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

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024
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

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024

Wordcount for paper.md is 2535

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024
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

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024

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

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

Hi @rashatwi how is your review going?

from joss-reviews.

mastricker avatar mastricker commented on August 20, 2024

@editorialbot generate my checklist

from joss-reviews.

mastricker avatar mastricker commented on August 20, 2024

** General checks

  • Repository
  • License
  • Contribution and authorship

** Functionality

  • Installation

  • Functionality

  • Performance

    I have not checked this explicitly, but base my assessment on the
    neighborlist example presented in the accompanying paper. This
    reflects my own experience with ASE neighborlists and the need to
    provide an own implementation for efficient generation of neighbor
    lists.

** Documentation

  • Statement of need

  • Installation instructions

  • Example usage

  • Functionality documentation

    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.

  • Automated tests

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

    Presumably test_energy_release.py provides additional s as output

    On commit c0bee217596e2ef7e610ea16b3e8a3a7b3f1bce1, running the
    tests as per the documentation in the repo python -m pytest .
    results in a segmentation fault in test_neighbours.py -> Fatal Python error: Segmentation fault
    I'm using Python 3.10.13 on Debian 12 (trixie). Full traceback for is

  test_neighbours.py ..............Fatal Python error: Segmentation fault

  Current thread 0x00007fd1800df740 (most recent call first):
  File "/home/markus/Documents/Reviews/2023-10-15_JOSS_MatSciPy/matscipy/tests/test_neighbours.py", line 289 in test_triplet_list
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/unittest/case.py", line 549 in _callTestMethod
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/unittest/case.py", line 591 in run
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/unittest/case.py", line 650 in __call__
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/_pytest/unittest.py", line 333 in runtest
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/_pytest/runner.py", line 169 in pytest_runtest_call
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/pluggy/_callers.py", line 77 in _multicall
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/pluggy/_manager.py", line 115 in _hookexec
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/pluggy/_hooks.py", line 493 in __call__
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/_pytest/runner.py", line 262 in <lambda>
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/_pytest/runner.py", line 341 in from_call
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/_pytest/runner.py", line 261 in call_runtest_hook
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/_pytest/runner.py", line 222 in call_and_report
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/_pytest/runner.py", line 133 in runtestprotocol
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/_pytest/runner.py", line 114 in pytest_runtest_protocol
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/pluggy/_callers.py", line 77 in _multicall
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/pluggy/_manager.py", line 115 in _hookexec
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/pluggy/_hooks.py", line 493 in __call__
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/_pytest/main.py", line 350 in pytest_runtestloop
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/pluggy/_callers.py", line 77 in _multicall
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/pluggy/_manager.py", line 115 in _hookexec
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/pluggy/_hooks.py", line 493 in __call__
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/_pytest/main.py", line 325 in _main
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/_pytest/main.py", line 271 in wrap_session
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/_pytest/main.py", line 318 in pytest_cmdline_main
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/pluggy/_callers.py", line 77 in _multicall
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/pluggy/_manager.py", line 115 in _hookexec
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/pluggy/_hooks.py", line 493 in __call__
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/_pytest/config/__init__.py", line 169 in main
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/_pytest/config/__init__.py", line 192 in console_main
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/site-packages/pytest/__main__.py", line 5 in <module>
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/runpy.py", line 86 in _run_code
  File "/home/markus/miniconda3/envs/matscipy_review/lib/python3.10/runpy.py", line 196 in _run_module_as_main
  • Community guidelines

** Software paper

  • Summary
  • Statement of need
  • State of the field
  • Quality of writing
  • References

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.

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

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

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024

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

from joss-reviews.

mbarzegary avatar mbarzegary commented on August 20, 2024

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.

from joss-reviews.

mbarzegary avatar mbarzegary commented on August 20, 2024

Review checklist for @mbarzegary

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

General checks

  • Repository: Is the source code for this software available at the https://github.com/libAtoms/matscipy?
  • 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 (@pastewka) 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?

from joss-reviews.

pgrigorev avatar pgrigorev commented on August 20, 2024

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.

from joss-reviews.

rashatwi avatar rashatwi commented on August 20, 2024

Review checklist for @rashatwi

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

General checks

  • Repository: Is the source code for this software available at the https://github.com/libAtoms/matscipy?
  • 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 (@pastewka) 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?

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024

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

from joss-reviews.

rashatwi avatar rashatwi commented on August 20, 2024

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

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

@pastewka please have a look at the comments above.

from joss-reviews.

mastricker avatar mastricker commented on August 20, 2024

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.

from joss-reviews.

pgrigorev avatar pgrigorev commented on August 20, 2024

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

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

@pgrigorev could you finish the requested changes?

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

Hi @mbarzegary how is your review going?

from joss-reviews.

mbarzegary avatar mbarzegary commented on August 20, 2024

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

from joss-reviews.

pgrigorev avatar pgrigorev commented on August 20, 2024

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

from joss-reviews.

pgrigorev avatar pgrigorev commented on August 20, 2024

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

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

@mastricker could you please have a look?

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

Hi @rashatwi how is your review going?

from joss-reviews.

mastricker avatar mastricker commented on August 20, 2024

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

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

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

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

Hi @rashatwi how is your review going?

from joss-reviews.

rashatwi avatar rashatwi commented on August 20, 2024

@editorialbot generate pdf

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024

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

from joss-reviews.

rashatwi avatar rashatwi commented on August 20, 2024

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.

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024

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.

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

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

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024

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

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

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

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024

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

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024

@editorialbot generate pdf

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024

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

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024

@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 test_dislocation.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 test_neighbours.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.fracture_mechanics.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 fracture_mechanics 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_solver_fenics 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.

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

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

from joss-reviews.

mastricker avatar mastricker commented on August 20, 2024

@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

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024

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

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

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

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

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

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

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

from joss-reviews.

mbarzegary avatar mbarzegary commented on August 20, 2024

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.

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

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

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

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

@editorialbot check references

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

@editorialbot generate pdf

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024
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.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
- 10.12688/openreseurope.14445.2 is OK

MISSING DOIs

- 10.1007/978-3-319-06898-5_4 may be a valid DOI for title: Co-simulations of Discrete and Finite Element Codes

INVALID DOIs

- None

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

@pastewka Please check the missing DOI


MISSING DOIs

- 10.1007/978-3-319-06898-5_4 may be a valid DOI for title: Co-simulations of Discrete and Finite Element Code

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024

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

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

@pastewka Can you please take a look at my comments?

  • For Figure 1, I could not find the hardware details, the code was tested and the version. Can this be added?
  • Neighbour list. An efficient linear-scaling neighbour list implemented in C delivers orders-
154
 of-magnitude faster performance for large systems than the pure Python implementation
155
 in ASE (Larsen et al., 2017), see Figure 1.
  • Committee models.. - It seems here are two dots, please verify

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024

@editorialbot check references

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024
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.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
- 10.12688/openreseurope.14445.2 is OK
- 10.1007/978-3-319-06898-5_4 is OK

MISSING DOIs

- None

INVALID DOIs

- None

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024

@editorialbot generate pdf

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024

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

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024

@pastewka Can you please take a look at my comments?

  • For Figure 1, I could not find the hardware details, the code was tested and the version. Can this be added?
  • Neighbour list. An efficient linear-scaling neighbour list implemented in C delivers orders-
154
 of-magnitude faster performance for large systems than the pure Python implementation
155
 in ASE (Larsen et al., 2017), see Figure 1.
  • Committee models.. - It seems here are two dots, please verify

@diehlpk I've updated the paper, can you check if your comments are addressed?

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

@pastewka thanks, please do the following

  • Please generate a new release, including all changes made during the review. Post the version number here.
  • Please upload the release to Zenodo and make sure the title and authors match with the paper. Post the DOI here.

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024

@diehlpk PS: Here is a Zenodo preview, but I don't know if this is visible without my credentials: https://zenodo.org/records/10564956?preview=1

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024

@diehlpk PS: I release the Zenodo record, I realized I can always update the metadata later.

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024

URL is https://doi.org/10.5281/zenodo.10564956

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

@editorialbot set 10.5281/zenodo.10564956 as archive

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

@editorialbot set v1.0.0 as version

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024

Done! version is now v1.0.0

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024

Done! archive is now 10.5281/zenodo.10564956

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

@pastewka It looks good and I can recommend the paper for acceptance.

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

@editorialbot recommend-accept

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024
Attempting dry run of processing paper acceptance...

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024
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.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
- 10.12688/openreseurope.14445.2 is OK
- 10.1007/978-3-319-06898-5_4 is OK

MISSING DOIs

- None

INVALID DOIs

- None

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024

⚠️ Error preparing paper acceptance. The generated XML metadata file is invalid.

Element isbn: [facet 'minLength'] The value has a length of '9'; this underruns the allowed minimum length of '10'.

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

@pastewka can you please check the isbn in your Bibtex file?

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024

@diehlpk Fixed it. It was an ISSN, not an ISBN

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

@editorialbot recommend-accept

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024
Attempting dry run of processing paper acceptance...

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024
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.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
- 10.12688/openreseurope.14445.2 is OK
- 10.1007/978-3-319-06898-5_4 is OK

MISSING DOIs

- None

INVALID DOIs

- None

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024

⚠️ Error preparing paper acceptance. The generated XML metadata file is invalid.

Element inline-formula is not declared in institution list of possible children
ID ref-Jana2019 already defined

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

@pastewka Sorry, again another issue. Please take a look?

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024

@diehlpk I've fixed the duplicate reference (not yet merged to master) but I don't know what "Element inline-formula is not declared in institution list of possible children" refers to.

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024

@editorialbot generate pdf

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024

@diehlpk We have removed all abstracts from the .bib file, which I suppose may have been the source of the above error.

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024

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

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

@editorialbot recommend-accept

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024
Attempting dry run of processing paper acceptance...

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024
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.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.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
- 10.12688/openreseurope.14445.2 is OK
- 10.1007/978-3-319-06898-5_4 is OK

MISSING DOIs

- None

INVALID DOIs

- None

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024

⚠️ Error preparing paper acceptance. The generated XML metadata file is invalid.

Element inline-formula is not declared in institution list of possible children

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

@pastewka Still not working, I am asking the developers.

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024

@diehlpk Okay, thanks!

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

@xuanxu or @arfon could you please have a look, what is going wrong?

from joss-reviews.

xuanxu avatar xuanxu commented on August 20, 2024

The error is generated because the JATS metadata file used to deposit the paper does not allow inline math formulas in the headers, so this mu is causing it to fail.
Is it possible to spell that name in any other way?

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024

@xuanxu Does the system support UTF-8?

from joss-reviews.

xuanxu avatar xuanxu commented on August 20, 2024

@pastewka Yes! Good idea! Merging this PR should fix the error: libAtoms/matscipy#235

from joss-reviews.

pastewka avatar pastewka commented on August 20, 2024

@diehlpk I've merged @xuanxu's PR... Hopefully it works now. Many thanks to both of you!

from joss-reviews.

diehlpk avatar diehlpk commented on August 20, 2024

@editorialbot recommend-accept

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024
Attempting dry run of processing paper acceptance...

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024
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.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.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
- 10.12688/openreseurope.14445.2 is OK
- 10.1007/978-3-319-06898-5_4 is OK

MISSING DOIs

- None

INVALID DOIs

- None

from joss-reviews.

editorialbot avatar editorialbot commented on August 20, 2024

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

Check final proof 👉📄 Download article

If the paper PDF and the deposit XML files look good in openjournals/joss-papers#4955, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

from joss-reviews.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.