[REVIEW]: The o80 C++ templated toolbox: Designing customized Python APIs for synchronizing realtime processes
whedon opened this issue · 106 comments
Submitting author: @vincentberenz (Vincent Berenz)
Repository: https://github.com/intelligent-soft-robots/o80
Version: v1.0
Editor: @gkthiruvathukal
Reviewers: @traversaro, @vissarion
Archive: 10.5281/zenodo.5357876
Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.
Status
Status badge code:
HTML: <a href="https://joss.theoj.org/papers/828f23ed755fafd07add9e6154b76b65"><img src="https://joss.theoj.org/papers/828f23ed755fafd07add9e6154b76b65/status.svg"></a>
Markdown: [](https://joss.theoj.org/papers/828f23ed755fafd07add9e6154b76b65)
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
@vissarion & @traversaro, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:
- Make sure you're logged in to your GitHub account
- Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @gkthiruvathukal 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 ✨
Review checklist for @vissarion
Conflict of interest
- I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.
Code of Conduct
- I confirm that I read and will adhere to the JOSS code of conduct.
General checks
- Repository: Is the source code for this software available at the repository url?
- 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 (@vincentberenz) 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
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: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
- 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?
Review checklist for @traversaro
Conflict of interest
- I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.
Code of Conduct
- I confirm that I read and will adhere to the JOSS code of conduct.
General checks
- Repository: Is the source code for this software available at the repository url?
- 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 (@vincentberenz) 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
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: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
- 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?
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @giodegas, @traversaro it looks like you're currently assigned to review this paper 🎉.
Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.
⭐ 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:
- Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:
- 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 (note 'MISSING' DOIs are suggestions that need verification):
OK DOIs
- 10.1109/LRA.2018.2795645 is OK
- 10.1109/iros.2012.6386109 is OK
MISSING DOIs
- None
INVALID DOIs
- None
Hi @vincentberenz , I see that you submitted the version o80 v1.0 for review, but I can't find it in the repo any reference to any v1.0 version, can you clarify this? Thanks!
Hi @traversaro , thanks a lot for accepting to review the paper. My apologies, I missed that a tagged/version had to set for review. The master branch is meant to always be the latest stable version. If needed, I'd be happy to tag the latest commit as version 1.0.
My apologies, I missed that a tagged/version had to set for review.
To be honest, I did not saw any requirement on this in the JOSS guidelines as well, but given that the review issue ( #2752 (comment) ) clearly stated that the version was the 1.0, so I was just confused about that. Perhaps @gkthiruvathukal can help us on this?
Hi @traversaro , thanks a lot for accepting to review the paper. My apologies, I missed that a tagged/version had to set for review. The master branch is meant to always be the latest stable version. If needed, I'd be happy to tag the latest commit as version 1.0.
Besides the specific requirement of the JOSS review, it seems to be that at least the repos https://github.com/intelligent-soft-robots/o80 are https://github.com/intelligent-soft-robots/o80_example are relatively tightly coupled (one is the software and one is the canonical example on how to use it). Unless you ensure perpetual API stability (that I could not see in the docs) as a user if I start developing against a given version of o80 it would be useful to know what is the corresponding version of https://github.com/intelligent-soft-robots/o80_example . This information probably can be obtained looking at the commit dates, but having a clear version to couple the two could be convenient for the user.
Related to installation instructions, I opened an issue in the repo: intelligent-soft-robots/o80#2 .
@traversaro I think we're ok here and that review can proceed, right? I think everything being reviewed should be tagged v1.0. You raise good points about the coupled example. Even though that might not show up in the submission metadata, it should be tagged v1.0, too. I think it is safe to proceed with review nevertheless. At the end, based on all issues raised, the authors will probably make some changes and tag everything again at the end. Hope that helps!
Thanks, that is clear to me. For sure the review can continue and is not blocked by this, I just wanted to clarify things.
Thanks, @traversaro. Really glad to have your help!
@traversaro @gkthiruvathukal I tagged both o80 and o80_example with version 1.0. Thanks for the comments on the alignment of versions between o80 and o80_example, as well as for the issue related to the installation procedure and the broken links.
This is my first submission to JOSS: what is the recommended way ? Answering comments/issues as they are arise, in an interactive fashion, or wait for the review to be done before addressing the issues ?
This is my first submission to JOSS: what is the recommended way ? Answering comments/issues as they are arise, in an interactive fashion, or wait for the review to be done before addressing the issues ?
It is the first review of JOSS as well. I don't know if there is any recommendation on that, but looking at the other reviews, it seems that an interactive review seems to be possible. Personally I think that interactive review would be desirable, for example because I wanted to test the example, and I already found problems in following the docs for installation, so for me it would be ideal if the installation docs could be fixed, so that I could then try the example following the new installation docs.
This is my first submission to JOSS: what is the recommended way ? Answering comments/issues as they are arise, in an interactive fashion, or wait for the review to be done before addressing the issues ?
It is the first review of JOSS as well. I don't know if there is any recommendation on that, but looking at the other reviews, it seems that an interactive review seems to be possible. Personally I think that interactive review would be desirable, for example because I wanted to test the example, and I already found problems in following the docs for installation, so for me it would be ideal if the installation docs could be fixed, so that I could then try the example following the new installation docs.
Interactive review sounds also great to me. I'll do so, and I will fix the docs soon.
@giodegas and @traversaro, can you provide a brief update on the status of your reviews?
Hi @gkthiruvathukal, I provided some preliminary comments to @vincentberenz on the installation procedure, that @vincentberenz addressed, now I need to install the software using the new docs, and play a bit with the examples to go forward with the review.
I apologize, but over the last months I could not find any free time to concentrate on this task, as I hoped.
So I am refusing to review this work considering the back-log of my primary job duties.
@giodegas I totally understand. These are tough times for all of us. I'll see if I can add someone else.
@vincentberenz I apologize for the inconvenience but could you take another look at the list of reviewers and see if there is anyone else we could add as a second reviewer? I may need you to suggest 2-3 possibilities.
I can suggest @PatrizioM , one of our PhD students, but he is also quite busy at the moment. Ask him.
@gkthiruvathukal no pb, here a list: jcarpent , davidbeckingsale , sarats , marcoapintoo, gcdeshpande , vissarion , vijaysm
Apologies for the delay in getting a replacement reviewer.
@davidbeckingsale or @vissarion Would you be willing to contribute a review for this JOSS submission?
@gkthiruvathukal I am OK with reviewing this submission. Is 4-5 weeks ok for a deadline?
Thanks for such a quick response, @vissarion. I'm willing to work with your schedule. We're still in a COVID-19 world, and your timeframe sounds perfect to me. I'm going to add you now.
@whedon add @vissarion as reviewer
OK, @vissarion is now a reviewer
@whedon commands
Here are some things you can ask me to do:
# List all of Whedon's capabilities
@whedon commands
# Assign a GitHub user as the sole reviewer of this submission
@whedon assign @username as reviewer
# Add a GitHub user to the reviewers of this submission
@whedon add @username as reviewer
# Re-invite a reviewer (if they can't update checklists)
@whedon re-invite @username as reviewer
# Remove a GitHub user from the reviewers of this submission
@whedon remove @username as reviewer
# List of editor GitHub usernames
@whedon list editors
# List of reviewers together with programming language preferences and domain expertise
@whedon list reviewers
# Change editorial assignment
@whedon assign @username as editor
# Set the software archive DOI at the top of the issue e.g.
@whedon set 10.0000/zenodo.00000 as archive
# Set the software version at the top of the issue e.g.
@whedon set v1.0.1 as version
# Open the review issue
@whedon start review
EDITORIAL TASKS
# All commands can be run on a non-default branch, to do this pass a custom
# branch name by following the command with `from branch custom-branch-name`.
# For example:
# Compile the paper
@whedon generate pdf
# Compile the paper from alternative branch
@whedon generate pdf from branch custom-branch-name
# Remind an author or reviewer to return to a review after a
# certain period of time (supported units days and weeks)
@whedon remind @reviewer in 2 weeks
# Ask Whedon to do a dry run of accepting the paper and depositing with Crossref
@whedon accept
# Ask Whedon to check the references for missing DOIs
@whedon check references
# Ask Whedon to check repository statistics for the submitted software
@whedon check repository
EiC TASKS
# Invite an editor to edit a submission (sending them an email)
@whedon invite @editor as editor
# Reject a paper
@whedon reject
# Withdraw a paper
@whedon withdraw
# Ask Whedon to actually accept the paper and deposit with Crossref
@whedon accept deposit=true
Apologies for the delay in getting a replacement reviewer.
@davidbeckingsale or @vissarion Would you be willing to contribute a review for this JOSS submission?
I don't have the bandwidth right now. Thanks for asking.
@traversaro and @vissarion, happy new year! Can. you let me know where you are with respect to the review for this JOSS submission?
@traversaro and @vissarion, happy new year! Can. you let me know where you are with respect to the review for this JOSS submission?
Hi @gkthiruvathukal ! Thanks for the ping. I plan to resume the review this week.
@traversaro Much appreciated!!
@gkthiruvathukal happy new year! I paused my review waiting for a feedback in intelligent-soft-robots/o80#3 since it seems there are some works in progress on the package.
@gkthiruvathukal thanks for pinging this !
@vissarion @traversaro : indeed, I am working on the packaging. For the build system, we are moving from catkin (used by ROS) to colcon/ament (used by ROS2). In parallel, I am creating a ppa to make installation of the compiled software trivial. All this is taking much more time and effort than expected. And also not as smooth as it should, you may notice the tag for compilation, unit tests and doc building are all red ... Obviously, this make reviewing much harder than it should (but it can wait, if ok with you). My apologies, switch to colcon and setup of a ppa were not part of the plan yet when I requested the review. Note that on the other hand the source code is not expected to change (beyond possible bug fixes).
@vincentberenz So can you give us a sense how long you'll need to make these changes? I can nudge everyone in a couple of weeks.
@gkthiruvathukal I'd say around 2 weeks. I will ping this thread once up and running
@vincentberenz Just a nudge to see how things are going with the review. We greatly appreciate your help to complete it.
@gkthiruvathukal things are progressing, but taking more time than expected. My apologies for making this review difficult.
@vincentberenz No worries here. Thank you for responding!
@gkthiruvathukal @traversaro @vissarion
The software is ready for the review to resume. Apologies again for this unexpected (long !) delay. The continuous integration has been fully refactored and now generates binaries and source tar balls. Here is the new documentation page:
http://people.tuebingen.mpg.de/mpi-is-software/o80/docs/o80/index.html
the installation page has been updated:
http://people.tuebingen.mpg.de/mpi-is-software/o80/docs/o80/doc/02.installation.html
Thanks, @vincentberenz. @traversaro and @vissarion, please let me know when you have a chance to take a look at the revisions.
Hi @vincentberenz, thanks for the feedback, I will look into that the next week.
Hi, I did another review pass after the revision and left a few comments as issues in the target repository:
@vissarion Thanks a lot for your time and the comments. I agreed with all of them, and did my best to address them all. Please see answers in the issues.
@whedon generate pdf
@vissarion and @traversaro Can you give me a sense of whether @vincentberenz responses are sufficient? It seems like we are getting closer to acceptance.
@vincentberenz thanks for your replies.
@gkthiruvathukal I am OK from my side.
I plan to check the @vincentberenz replies are ok during this week.
@whedon generate pdf
PDF failed to compile for issue #2752 with the following error:
/app/vendor/bundle/ruby/2.6.0/bundler/gems/whedon-92346a0773a4/lib/whedon.rb:204:in `block in parse_authors': Author (Vincent Berenz^[corresponding author]) is missing affiliation (RuntimeError)
from /app/vendor/bundle/ruby/2.6.0/bundler/gems/whedon-92346a0773a4/lib/whedon.rb:202:in `each'
from /app/vendor/bundle/ruby/2.6.0/bundler/gems/whedon-92346a0773a4/lib/whedon.rb:202:in `parse_authors'
from /app/vendor/bundle/ruby/2.6.0/bundler/gems/whedon-92346a0773a4/lib/whedon.rb:93:in `initialize'
from /app/vendor/bundle/ruby/2.6.0/bundler/gems/whedon-92346a0773a4/lib/whedon/processor.rb:38:in `new'
from /app/vendor/bundle/ruby/2.6.0/bundler/gems/whedon-92346a0773a4/lib/whedon/processor.rb:38:in `set_paper'
from /app/vendor/bundle/ruby/2.6.0/bundler/gems/whedon-92346a0773a4/bin/whedon:58:in `prepare'
from /app/vendor/bundle/ruby/2.6.0/gems/thor-0.20.3/lib/thor/command.rb:27:in `run'
from /app/vendor/bundle/ruby/2.6.0/gems/thor-0.20.3/lib/thor/invocation.rb:126:in `invoke_command'
from /app/vendor/bundle/ruby/2.6.0/gems/thor-0.20.3/lib/thor.rb:387:in `dispatch'
from /app/vendor/bundle/ruby/2.6.0/gems/thor-0.20.3/lib/thor/base.rb:466:in `start'
from /app/vendor/bundle/ruby/2.6.0/bundler/gems/whedon-92346a0773a4/bin/whedon:131:in `<top (required)>'
from /app/vendor/bundle/ruby/2.6.0/bin/whedon:23:in `load'
from /app/vendor/bundle/ruby/2.6.0/bin/whedon:23:in `<main>'
@whedon generate pdf
I made a minor change in the paper: I updated the affiliations so that they align with some new guidelines of our institute
@whedon generate pdf
@vissarion and @traversaro Can you give me a sense of whether @vincentberenz latest updates are sufficient? The earlier correspondence suggests we are good to go. I'd like to move to accept this paper by end of week if there are no additional issues to address.
Thanks for pinging !
I still have this issue to address:
Thanks, @vincentberenz. Please do follow up when complete so we can take the next steps!
@gkthiruvathukal @vissarion @traversaro It seems to me all issues have been addressed
@gkthiruvathukal @vissarion @traversaro It seems to me all issues have been addressed
Thanks, I will check this during the next week!
Thanks @vincentberenz for addressing my comments! @gkthiruvathukal The review is ok from my side.
@traversaro Thanks for making sure your comments have been addressed. I'll be moving toward acceptance. There are some final steps to complete. Stay tuned.
Hi @vincentberenz, please do the following:
Please do the following:
- Make a tagged release of your software, and list the version tag of the archived version here.
- Archive the reviewed software in Zenodo
- Check the Zenodo deposit has the correct metadata, this includes the title (should match the paper title) and author list (make sure the list is correct and people who only made a small fix are not on it); you may also add the authors' ORCID.
- List the Zenodo DOI of the archived version here.
Hi @traversaro, please do the following:
I guess the message was intended for @vincentberenz ?
@traversaro Yes! That was the intention, but my fingers mentioned you instead of @vincentberenz. Thanks! I corrected it above.
@traversaro @vissarion thanks again for your time and your useful comments
A robotic group of the University of Aachen just finished developing o80 drivers for the Franka robots (which are currently quite popular in academia). I would like to update the paper with a reference to their repo. They told me it would make public very soon. As soon as this happens, I would update the paper accordingly (minimal change, smthg like "o80 drivers for the Franka robots are also availalbe [reference]"), and then go through the todo list you provided.
@vincentberenz This is absolutely fine. Please signal us when you have made the updates and have completed the checklist. The change you are proposing seems routine, so we do not need to do a full re-review by any means. Just make sure the article PDF generates properly!
Looks like it may took a bit more time before the drivers or the franka robot get released, so going forward now:
https://zenodo.org/record/5357876#.YS9gj1uxUUE
- I created the annoter tag o80_joss for the repos: o80, o80_example, mpi_cmake_modules, real_time_tools, serialization_utils, shared_memory, signal_handler and time_series (i.e. o80 and all dependencies). The tar ball uploaded to zenodo contains the source code of all these repos.
- the DOI: 10.5281/zenodo.5357876
@whedon generate pdf
The o80's drivers for the Franka robot have been made open source, and I updated the paper to refer to them
👋 @gkthiruvathukal - what's the next step here?
@vincentberenz I had already recommended this for acceptance. So can you go back to my August 1st comment and make sure to provide the needed information so we can bring this submission to closure?
@gkthiruvathukal I believe I already went through all the points, see my comment from Sept 1st.
@vincentberenz, So there are no changes to the code based on your point about the o80 drivers, right?
Thanks for your prompt response.
@gkthiruvathukal I confirm the only change is the update of the paper (referring to the franka panda drivers). None of the code has been changed.
@vincentberenz So I can just use your v1.0 tag for the release, right? (Never mind, I found it.)
@whedon set 10.5281/zenodo.5357876 as archive
OK. 10.5281/zenodo.5357876 is the archive.
@whedon set v1.0 as version
OK. v1.0 is the version.
@vincentberenz So I can just use your v1.0 tag for the release, right?
yes
@whedon accept
To recommend a paper to be accepted use @whedon recommend-accept
@whedon recommend-accept
Attempting dry run of processing paper acceptance...
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):
OK DOIs
- 10.1109/LRA.2018.2795645 is OK
- 10.1109/iros.2012.6386109 is OK
MISSING DOIs
- None
INVALID DOIs
- None
👋 @openjournals/joss-eics, this paper is ready to be accepted and published.
Check final proof 👉 openjournals/joss-papers#2642
If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#2642, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.
@whedon accept deposit=true
👋 @vincentberenz - I've suggested some minor changes in the paper in intelligent-soft-robots/o80#11 - please merge these or let me know what you disagree with, then we can proceed to acceptance and publication.
@danielskatz Thanks for the improvements ! I merged the branch into master.
@whedon recommend-accept
Attempting dry run of processing paper acceptance...
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):
OK DOIs
- 10.1109/LRA.2018.2795645 is OK
- 10.1109/iros.2012.6386109 is OK
MISSING DOIs
- None
INVALID DOIs
- None
👋 @openjournals/joss-eics, this paper is ready to be accepted and published.
Check final proof 👉 openjournals/joss-papers#2643
If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#2643, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.
@whedon accept deposit=true
@whedon accept deposit=true