search

openjournals / joss-reviews

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

[REVIEW]: TriP: A Python package for the kinematic modeling of serial-parallel hybrid robots #3967

Closed whedon closed 2 years ago

whedon commented 2 years ago

Submitting author: !--author-handle-->@liquidcronos<!--end-author-handle-- (Jan Baumgärtner) Repository: https://github.com/TriPed-Robot/TriP Branch with paper.md (empty if default branch): paper Version: v1.0.3 Editor: !--editor-->@danielskatz<!--end-editor-- Reviewers: @SeungBack, @bmagyar Archive: 10.5281/zenodo.6360087

:warning: JOSS reduced service mode :warning:

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

Status badge code:

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

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

@SeungBack & @bmagyar, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. 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 @danielskatz 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 @SeungBack

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @bmagyar

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 2 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @SeungBack, @bmagyar it looks like you're currently assigned to review this paper :tada:.

:warning: JOSS reduced service mode :warning:

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.

:star: Important :star:

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:

watching

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

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
whedon commented 2 years ago 
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.06 s (540.6 files/s, 69444.1 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          15            409            532           1146
XML                              1              0              2            792
reStructuredText                 4            219            174            305
Markdown                         5             43              0             98
YAML                             3             17             18             78
make                             1              4              6             10
SVG                              1              0              0              1
-------------------------------------------------------------------------------
SUM:                            30            692            732           2430
-------------------------------------------------------------------------------

Statistical information for the repository '3c8e1b429fe35986dea22c43' was
gathered on 2021/12/01.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Jan Baumgärnter                 17           359            215            1.48
Jan Baumgärtner                102         17383           2868           52.37
Torben Miller                   80          2635          15208           46.14

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
Jan Baumgärnter            1791          498.9          2.1                7.09
Jan Baumgärtner             162            0.9          0.0               54.32
Torben Miller               134            5.1          6.3                7.46
whedon commented 2 years ago

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

 Can't find any papers to compile :-(
danielskatz commented 2 years ago

@whedon generate pdf from branch paper

whedon commented 2 years ago 
Attempting PDF compilation from custom branch paper. Reticulating splines etc...
danielskatz commented 2 years ago

@whedon check references from branch paper

whedon commented 2 years ago 
Attempting to check references... from custom branch paper
whedon commented 2 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/j.mechatronics.2020.102367 is OK
- 10.1007/s12532-018-0139-4 is OK
- 10.1109/TMECH.2006.871087 is OK
- 10.1016/j.mechmachtheory.2008.03.002 is OK
- 10.1016/j.rcim.2012.09.016 is OK
- 10.1109/MRA.2011.2181749 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 2 years ago

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

danielskatz commented 2 years ago

👋 @SeungBack and @bmagyar - Thanks for agreeing to review this submission. This is the review thread for the paper. All of our communications will happen here from now on.

Both reviewers have checklists at the top of this thread with the JOSS requirements. As you go over the submission, please check any items that you feel have been satisfied. There are also links to the JOSS reviewer guidelines.

Please read the first couple of comments in this issue carefully, so that you can accept the invitation from JOSS and be able to check items, and so that you don't get overwhelmed with notifications from other activities in JOSS.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention openjournals/joss-reviews#3967 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for reviews to be completed within about 2-4 weeks. Please let me know if either of you require some more time. We can also use Whedon (our bot) to set automatic reminders if you know you'll be away for a known period of time.

Please feel free to ping me (@danielskatz) if you have any questions/concerns.

whedon commented 2 years ago

:wave: @bmagyar, please update us on how your review is going (this is an automated reminder).

whedon commented 2 years ago

:wave: @SeungBack, please update us on how your review is going (this is an automated reminder).

danielskatz commented 2 years ago

HI @bmagyar and @SeungBack - I've been on vacation, and continue to be for another week, but also want to check on the status of this review. I'll check again on the 10th when I'm back at work.

SeungBack commented 2 years ago

I reviewed the paper and documentation, and here are the comments. I am reviewing the codes and will add detailed comments about the codes soon.

This paper introduces an easy-to-use Python package for computing the forward and inverse kinematics of robots. The author claims that previous libraries lack support for hybrid serial-parallel systems, and TirP can be used to model both parallel and serial mechanisms. Solving Inverse and forward kinematics is an essential component for robotics, and I think this library can be useful for those who want to easily model the kinematics of complex robots.

Major issues

Minor issues

liquidcronos commented 2 years ago

Hi @SeungBack Thanks for your valuable feedback! I am not sure If I should immediately respond or wait for the second reviewer, So I hope that it is alright to address the Issues:

There are other libraries that support computing the F.K and I.K of robots, such as ikpy and Klampt, but the paper compares TriP with Openrave, MoveIt, and Matlab. It would be better to describe the strength of this library by comparing it to other libraries in terms of functionality and speed.

You are correct we do not mention other python kinematic libraries. This was an oversight, ultimately the libraries you mentioned suffer from the same shortcomings as the other ones namely lacking support for serial mechanisms or hybrid serial parallel mechanisms. I will add them shortly.

For the easy modeling of chain models, support of URDF import/export and 2D/3D visualization is necessary. Is there a plan to support this?

We completely agree that visualization is very important in practice. For this reason we are working on adding visualizations for a future release of the library.

While URDF is a standard format for robot descriptions, it is unfortunately rather basic and lacks support for the hybrid mechanisms TriP aims to model. Since it is still sufficient to describe the virtual open chain of a mechanism we are currently working on URDF support which we will hopefully finish next month.

I think the summary section does not summarize the paper, rather, it introduces the overall background.

You are right. To be honest I was a bit confused by the required content of this section since many other papers seem to include the background in order to describe the purpose of the software for a 'diverse, non-specialist audience'. Would you rather I shorten the overview or write more about TriP itself?

In the tutorial, some code references are broken and refer to the wrong lines of code

Examples do not include the codes for importing packages. Please add this information (e.g. import trip_kinematics, from math import radians)

Thanks for letting me know, should be fixed now :)

Documentation includes only TriPed robots, and I think adding an example for popular serial mechanisms (e.g., Panda, UR5) would be helpful.

Ideally we want more examples of hybrid serial parallel robots to better showcase TriPs performance as you can see here. Popular examples of such hybrid systems are the Kuka KR 40 PA or the fanuc-m410ic. The main reason we have not included these systems so far is that we want to independently verify that we have correctly implemented the models. Since we are the only library supporting hybrid mechanisms calculating reference values is very difficult. For this reason we have currently included the excavator robots as toy examples while we slowly add more examples. If you want we can prioritize the development of these robot models to get them out sooner.

The names of the pip package name (trip-kinematics) and paper title (TriP) do not match, and thus please consider adding an installation section on readthedocs.

Good point, we will add a getting started section including the installation instructions of the README.md.

The colors of Figures 1 and 3 in the paper are confusing, and it would be better to use contrastive colors.

Are you referring only to the two green colors, or the blue and green color as well? I will try to change them to be more contrastive.

danielskatz commented 2 years ago

@liquidcronos - it's fine (and even good) for you to respond as issues come up - this will make it easier for the second reviewer. thanks!

liquidcronos commented 2 years ago

@SeungBack I have included IKPy and Klam't in my discussion. Let me know if this is sufficient or if you want a more detailed comparison

liquidcronos commented 2 years ago

@whedon generate pdf from branch paper

whedon commented 2 years ago 
Attempting PDF compilation from custom branch paper. Reticulating splines etc...
whedon commented 2 years ago

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

SeungBack commented 2 years ago

Hi @liquidcronos,

I have checked the updated version of the manuscripts, and I am satisfied with the response and the revised manuscript. It is good to hear that TriP will be updated to support URDF and visualization. Here are the additional comments for your response:

  1. Would you rather I shorten the overview or write more about TriP itself?

I recommend adding more about TriP itself (e.g., features, structures, and the strength of TriP).

  1. Are you referring only to the two green colors, or the blue and green colors as well?

The colors of green and blue links are hard to distinguish for me. 

  1. Examples do not include the codes for importing packages. Please add this information (e.g. import trip_kinematics, from math import radians)

    Thanks for letting me know, should be fixed now :)

Please let me know after updating the codes so that I can run and review them.

liquidcronos commented 2 years ago

Hey @SeungBack I have amended the paper with more references to TriPs features. As promised I have also added a getting started section that explains how the library is installed. The code examples should now also be self-contained and include all necessary imports.

Are there any other changes you would like to see apart from the figure colors (which I should be able to change until Friday)?

liquidcronos commented 2 years ago

@whedon generate pdf from branch paper

whedon commented 2 years ago 
Attempting PDF compilation from custom branch paper. Reticulating splines etc...
whedon commented 2 years ago

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

danielskatz commented 2 years ago

👋 @bmagyar - how is your review coming?

SeungBack commented 2 years ago

Hi @liquidcronos,

Examples do not include the codes for importing packages. Please add this information (e.g. import trip_kinematics, from math import radians)

I still can not find the codes for importing other libraries in Tutorial section. Where can I find the information about it?

liquidcronos commented 2 years ago

You are right, the imports for the tutorial where still missing, I only looked at the examples not the tutorials. It should be fixed now

danielskatz commented 2 years ago

Hi - I'm just checking in on this review.

@bmagyar - it doesn't look like you've gotten started on your checklist. How are things going?

@SeungBack - thanks for your progress so far.

@liquidcronos - thanks for responding to issues/comments

Let me know if there are any problems I can help with.

bmagyar commented 2 years ago

Hey guys, sorry it took so long but life got in the way... Will be working on the review now

danielskatz commented 2 years ago

👋 @bmagyar - How is your review going? Is there anything either I or @liquidcronos can do to help you at this point?

danielskatz commented 2 years ago

FYI - @bmagyar touched base via email and is on this.

liquidcronos commented 2 years ago

Thanks for the update!

bmagyar commented 2 years ago

Great work guys!

I really only have minor things related to docs and the examples: https://github.com/TriPed-Robot/trip_kinematics/issues/72 <-- this has been pending for 12 days or so https://github.com/TriPed-Robot/trip_kinematics/issues/74 <-- this one being the main one https://github.com/TriPed-Robot/trip_kinematics/pull/73 <-- miniscule fix submitted by me ;)

danielskatz commented 2 years ago

thanks @bmagyar

liquidcronos commented 2 years ago

Thanks for your review @bmagyar ! I seem to have set my notification settings wrong, because I wasn't informed of your Issues by Mail. This should be rectified now.

I will fix the type you found in https://github.com/TriPed-Robot/trip_kinematics/issues/72 right away. Thanks also for spotting the typo in the install instructions, I have approved your pull request and even found another typo in the same section...

Regarding your Issues with the tutorials, I have addressed them in the issue. Please let me know if these fixed are sufficient or if you still want more detailed instructions on the required version of the tutorial.

danielskatz commented 2 years ago

@bmagyar wrote to me that he "will need a few more days to double check the review responses"

liquidcronos commented 2 years ago

Hey, @danielskatz, thanks for the update!

danielskatz commented 2 years ago

👋 @bmagyar - any update? We're so close to done...

bmagyar commented 2 years ago

All green from my side too. Apologies for the large delay, I had a daughter born in the mean time :)

danielskatz commented 2 years ago

pesky babies :) More seriously, thanks!!

danielskatz commented 2 years ago

@liquidcronos - I'm going to proofread the paper next

danielskatz commented 2 years ago

@editorialbot check references

editorialbot commented 2 years ago

Checking the BibTeX entries failed with the following error: 

No paper file path
danielskatz commented 2 years ago

@editorialbot generate pdf

danielskatz commented 2 years ago

@editorialbot set paper as branch

editorialbot commented 2 years ago

Done! branch is now paper

editorialbot commented 2 years ago

:warning: An error happened when generating the pdf. Paper file not found.

danielskatz commented 2 years ago

@editorialbot check references

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

OK DOIs

- 10.1016/j.mechatronics.2020.102367 is OK
- 10.1007/s12532-018-0139-4 is OK
- 10.1109/TMECH.2006.871087 is OK
- 10.1016/j.mechmachtheory.2008.03.002 is OK
- 10.1016/j.rcim.2012.09.016 is OK
- 10.1109/MRA.2011.2181749 is OK

MISSING DOIs

- None

INVALID DOIs

- None