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

⭐ Important ⭐

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

2. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

For a list of things I can do to help you, just type:

@whedon commands

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

@whedon generate pdf

Reference check summary:

OK DOIs

- 10.1016/j.cma.2019.04.006 is OK
- 10.1016/j.jmps.2018.02.015 is OK
- 10.1016/0890-6955(92)90064-N is OK
- 10.1063/1.1699786 is OK
- 10.1088/0953-8984/17/1/R01 is OK
- 10.1016/S0043-1648(99)00113-1 is OK
- 10.1029/2012GL054143 is OK
- 10.1007/s00466-017-1392-5 is OK
- 10.1016/j.ijengsci.2019.02.003 is OK
- 10.1007/s00466-018-1556-y is OK
- 10.1016/j.finel.2015.02.003 is OK
- 10.1115/1.2833523 is OK
- 10.1016/S0307-904X(79)80030-X is OK
- 10.1016/j.triboint.2017.04.023 is OK
- 10.1007/s11249-014-0395-z is OK
- 10.1016/j.ijsolstr.2014.09.019 is OK
- 10.1016/j.jmps.2017.07.016 is OK

MISSING DOIs

- https://doi.org/10.1103/physreve.86.035601 may be missing for title: Contact between Representative Rough Surfaces

INVALID DOIs

- None

👉 Check article proof 📄 👈

@whedon generate pdf

PDF failed to compile for issue #2121 with the following error:

Error reading bibliography ./paper.bibtex (line 232, column 3):
unexpected "j"
expecting space, ",", white space or "}"
Error running filter pandoc-citeproc:
Filter returned error status 1
Looks like we failed to compile the PDF

@whedon generate pdf

👉 Check article proof 📄 👈

👋 @likask, @agshvarts, @srmnitc, @chennachaos this is where the review takes place. Let me know if you have any questions. Thanks for your help!

Dear authors and reviewers

We wanted to notify you that in light of the current COVID-19 pandemic, JOSS has decided to suspend submission of new manuscripts and to handle existing manuscripts (such as this one) on a "best efforts basis". We understand that you may need to attend to more pressing issues than completing a review or updating a repository in response to a review. If this is the case, a quick note indicating that you need to put a "pause" on your involvement with a review would be appreciated but is not required.

Thanks in advance for your understanding.

Arfon Smith, Editor in Chief, on behalf of the JOSS editorial team.

@likask, @agshvarts, @srmnitc, @chennachaos - just a friendly check-in to see how things are going with your reviews?

Hi @arfon Sorry for the delay, I started looking into it before the lockdown, and then got carried away. I will get back to it now.

General checks

Contribution and authorship: The submitting author (@prs513rosewood) has clearly made the major contribution to the software (756 commits). However, regarding the list of authors, there is an AUTHORS file in the repository which lists two contributors who are not included as authors of the JOSS paper: Son Pham-Ba (49 commits) and Valentine Rey (18 commits). Furthermore, both of these contributors appeared as authors of a previous publication about this code on Zenodo (https://doi.org/10.5281/zenodo.3479237). Could the authors please justify the current choice of the authors' list for the JOSS paper and/or amend it?

@agshvarts On OS X go with Docker installation, is quite smooth. Use Dockerfile from the tamaas repository.

Hello @prs513rosewood, I run unit tests and code stuck with
build-release/tests/test_hertz_kato.py
How long this test suppose to take?

Hi @likask, for me this test is passing, but the one just after is failing, so that's probably why the code is stuck for you. I am on the current master branch and the commit message says that the tests are supposed to fail. To skip failing tests I run it as:
pytest build-release -v -k "not mindlin and not patch_plasticity"

Hi @likask @agshvarts and thanks for the feedback.

Hello @prs513rosewood, I run unit tests and code stuck with
build-release/tests/test_hertz_kato.py
How long this test suppose to take?

There was a problem with this test and docker (my Jenkins instance running the tests was also hanging on it, thanks to you I figured it was related to docker). I'm not sure what it was, but I refactored the test code and it seems to be working. That test is still kinda long (~30s) because the Stanley&Kato algorithm converges slowly to equilibrium.

Hi @likask, for me this test is passing, but the one just after is failing, so that's probably why the code is stuck for you. I am on the current master branch and the commit message says that the tests are supposed to fail. To skip failing tests I run it as:
pytest build-release -v -k "not mindlin and not patch_plasticity"

These tests should now be passing. I was adding some new features this week and did not mean to push the commits I was working on.

By the way, I made some changes to the way the main shared library is generated, so you may have to manually remove build-release/src/libTamaas.so.

I also simplified the installation process. Running scons prefix=/some/prefix install should install to a prefix, and running scons dev should run pip install --user -e like it was specified in the readme.

General checks

Contribution and authorship: The submitting author (@prs513rosewood) has clearly made the major contribution to the software (756 commits). However, regarding the list of authors, there is an AUTHORS file in the repository which lists two contributors who are not included as authors of the JOSS paper: Son Pham-Ba (49 commits) and Valentine Rey (18 commits). Furthermore, both of these contributors appeared as authors of a previous publication about this code on Zenodo (https://doi.org/10.5281/zenodo.3479237). Could the authors please justify the current choice of the authors' list for the JOSS paper and/or amend it?

We have, with permission of everyone involved, amended the list of authors. The original choice was to have as authors the people who contributed to the elastic-plastic part of Tamaas, but since this publication should be the reference point for the library as a whole, and the paper mentions parts contributed by Valentine Rey and Son Pham-Ba, we have decided to include them (with their permission) on this publication.

@whedon generate pdf

👉 Check article proof 📄 👈

@whedon check references

@srmnitc, @chennachaos thanks for your help with this review. This is just a friendly check-in to see how things are going with your reviews.

Reference check summary:

OK DOIs

- 10.1016/j.cma.2019.04.006 is OK
- 10.1016/j.jmps.2018.02.015 is OK
- 10.1016/0890-6955(92)90064-N is OK
- 10.1063/1.1699786 is OK
- 10.1088/0953-8984/17/1/R01 is OK
- 10.1016/S0043-1648(99)00113-1 is OK
- 10.1029/2012GL054143 is OK
- 10.1007/s00466-017-1392-5 is OK
- 10.1016/j.ijengsci.2019.02.003 is OK
- 10.1007/s00466-018-1556-y is OK
- 10.1016/j.finel.2015.02.003 is OK
- 10.1115/1.2833523 is OK
- 10.1016/S0307-904X(79)80030-X is OK
- 10.1016/j.triboint.2017.04.023 is OK
- 10.1103/PhysRevE.86.035601 is OK
- 10.1007/s11249-014-0395-z is OK
- 10.1016/j.ijsolstr.2014.09.019 is OK
- 10.1016/j.jmps.2017.07.016 is OK

MISSING DOIs

- None

INVALID DOIs

- None

@likask, @agshvarts, thanks for your review efforts!
It looks like @prs513rosewood has implemented some changes/replied to your comments. Can you follow up on these and resume the review? Thanks.

@prs513rosewood, thank you for amending the author's list and for fixing issues with Docker. Now the installation, automatic testing, and all examples are working fine in Docker, so I ticked corresponding checkboxes. Below are my comments regarding the documentation.

A statement of need. The authors are explaining clearly in the documentation the types of problems this software is aimed to solve. However, the target audience is not explicitly given, neither in the documentation nor in the paper. Could a short sentence describing the target audience be added to both documentation and the paper, which would make the purpose of this software more transparent for a broader computational mechanics/engineering community?

Functionality documentation. C++ core API is thoroughly documented, however API reference of python wrappers is missing (only headings are visible). Since the core functionality is expected to be accessed using python interface (all examples of solving typical problems use python API), could it be possible to add some documentation for python wrappers too? It seems to be possible with pybind11 using Sphinx, see here.

Thanks @agshvarts for the feedback. I've added the intended audience where suggested. For your second point, the Python API is documented via the autodoc feature of sphinx. This is done correctly if the docs are compiled locally (and the tamaas python module is installed). However, since that feature needs the compiled component of the Python API, it doesn't work on ReadTheDocs. I've tried to find a way to compile on RTD, but as far as I can tell they only allow their own Docker images to run builds. I've added dependencies for doc builds in the Dockerfile. I'll push as soon as c4science fixes the problems they're currently having.

@whedon generate pdf

PDF failed to compile for issue #2121 with the following error:

/app/vendor/ruby-2.4.4/lib/ruby/2.4.0/find.rb:43:in block in find': No such file or directory - tmp/2121 (Errno::ENOENT) from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/find.rb:43:in collect!'
from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/find.rb:43:in find' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-6225db5c4b57/lib/whedon/processor.rb:61:in find_paper_paths'
from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-6225db5c4b57/bin/whedon:50:in prepare' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/command.rb:27:in run'
from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/invocation.rb:126:in invoke_command' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor.rb:387:in dispatch'
from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/base.rb:466:in start' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-6225db5c4b57/bin/whedon:116:in <top (required)>'
from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in load' from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in

@whedon generate pdf

PDF failed to compile for issue #2121 with the following error:

/app/vendor/ruby-2.4.4/lib/ruby/2.4.0/find.rb:43:in block in find': No such file or directory - tmp/2121 (Errno::ENOENT) from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/find.rb:43:in collect!'
from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/find.rb:43:in find' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-6225db5c4b57/lib/whedon/processor.rb:61:in find_paper_paths'
from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-6225db5c4b57/bin/whedon:50:in prepare' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/command.rb:27:in run'
from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/invocation.rb:126:in invoke_command' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor.rb:387:in dispatch'
from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/base.rb:466:in start' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-6225db5c4b57/bin/whedon:116:in <top (required)>'
from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in load' from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in

@openjournals/dev can you help with these paper compilation issues? Thanks

I was going to check the paper source, but I can't current get to https://c4science.ch/source/tamaas/repository/master/ I get errors like

[cURL/7] (http://10.0.150.180:80/api/diffusion.browsequery) <CURLE_COULDNT_CONNECT> The cURL library raised an error while making a request. You may be able to find more information about this error (error code: 7) on the cURL site: http://curl.haxx.se/libcurl/c/libcurl-errors.html#CURLECOULDNTCONNECT

That would be why Whedon can't compile the paper then... 😁

From c4science:

IMPORTANT: 2020-05-29@16:30: We are experiencing huge performance issue and virtual machine lockup. Repositories operations are not possible at the moment.