New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REVIEW]: DART: Dynamic Animation and Robotics Toolkit #500

Closed
whedon opened this Issue Dec 12, 2017 · 52 comments

Comments

Projects
None yet
7 participants
@whedon
Collaborator

whedon commented Dec 12, 2017

Submitting author: @jslee02 (Jeongseok Lee)
Repository: https://github.com/dartsim/dart
Version: 6.3.0
Editor: @Kevin-Mattheus-Moerman
Reviewer: @costashatz
Archive: 10.5281/zenodo.1166142

Status

status

Status badge code:

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

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) 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


REVIEWER 1

@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.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @Kevin-Mattheus-Moerman know.

Conflict of interest

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?
  • Version: Does the release version given match the GitHub release (6.3.0)?
  • Authorship: Has the submitting author (@jslee02) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

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 function 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

  • Authors: Does the paper.md file include a list of authors with their affiliations?
  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

REVIEWER 2

@betatim, 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.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @Kevin-Mattheus-Moerman know.

Conflict of interest

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?
  • Version: Does the release version given match the GitHub release (6.3.0)?
  • Authorship: Has the submitting author (@jslee02) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

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 function 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

  • Authors: Does the paper.md file include a list of authors with their affiliations?
  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

REVIEWER 3

@costashatz, 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.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @Kevin-Mattheus-Moerman know.

Conflict of interest

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?
  • Version: Does the release version given match the GitHub release (6.3.0)?
  • Authorship: Has the submitting author (@jslee02) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

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 function 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

  • Authors: Does the paper.md file include a list of authors with their affiliations?
  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?
@whedon

This comment has been minimized.

Collaborator

whedon commented Dec 12, 2017

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks. @bmagyar it looks like you're currently assigned as the reviewer for 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:

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

This comment has been minimized.

Collaborator

whedon commented Dec 12, 2017

Attempting PDF compilation. Reticulating splines etc...
@Kevin-Mattheus-Moerman

This comment has been minimized.

Member

Kevin-Mattheus-Moerman commented Dec 12, 2017

@jslee02 @bmagyar @betatim let the reviewing begin!

@betatim you will have to accept the invitation to join the JOSS reviewers community: https://github.com/openjournals/joss-reviews/invitations before you will be able to check off items. Once you do so, I will also assign you to the GitHub issue. @arfon can you process the invitation?

Please use the two checklists above (one for each of you) to carry out your reviews.

An informal guideline is that we would like your review in 2 weeks, but sooner (or later) are also ok.

The reviewer guidelines are available here: http://joss.theoj.org/about#reviewer_guidelines

Please avoid lengthy details of difficulties in this review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) in this review thread. (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.)

Any questions/concerns, please let me know.

Thanks!!

@arfon

This comment has been minimized.

Member

arfon commented Dec 12, 2017

@betatim - you should have permissions to update this checklist once you accept the invite at https://github.com/openjournals/joss-reviews/invitations

@openjournals openjournals deleted a comment from whedon Dec 12, 2017

@arfon

This comment has been minimized.

Member

arfon commented Dec 12, 2017

@whedon generate pdf

@whedon

This comment has been minimized.

Collaborator

whedon commented Dec 12, 2017

Attempting PDF compilation. Reticulating splines etc...
@whedon

This comment has been minimized.

Collaborator

whedon commented Dec 12, 2017

https://github.com/openjournals/joss-papers/blob/joss.00500/joss.00500/10.21105.joss.00500.pdf

@openjournals openjournals deleted a comment from whedon Dec 12, 2017

@whedon

This comment has been minimized.

Collaborator

whedon commented Dec 12, 2017

https://github.com/openjournals/joss-papers/blob/joss.00500/joss.00500/10.21105.joss.00500.pdf
@bmagyar

This comment has been minimized.

Collaborator

bmagyar commented Jan 3, 2018

Almost all green from me, excellent submission 👍

@jslee02

This comment has been minimized.

jslee02 commented Jan 5, 2018

@whedon generate pdf

@whedon

This comment has been minimized.

Collaborator

whedon commented Jan 5, 2018

Attempting PDF compilation. Reticulating splines etc...
@whedon

This comment has been minimized.

Collaborator

whedon commented Jan 5, 2018

https://github.com/openjournals/joss-papers/blob/joss.00500/joss.00500/10.21105.joss.00500.pdf
@Kevin-Mattheus-Moerman

This comment has been minimized.

Member

Kevin-Mattheus-Moerman commented Jan 25, 2018

@whedon commands

@whedon

This comment has been minimized.

Collaborator

whedon commented Jan 25, 2018

Here are some things you can ask me to do:

# List all of Whedon's capabilities
@whedon commands

# Assign a GitHub user as the reviewer of this submission
@whedon assign @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

# Open the review issue
@whedon start review

🚧 🚧 🚧 Experimental Whedon features 🚧 🚧 🚧

# Compile the paper
@whedon generate pdf

@Kevin-Mattheus-Moerman

This comment has been minimized.

Member

Kevin-Mattheus-Moerman commented Jan 25, 2018

@whedon assign @costashatz as reviewer

@whedon

This comment has been minimized.

Collaborator

whedon commented Jan 25, 2018

OK, the reviewer is @costashatz

@Kevin-Mattheus-Moerman

This comment has been minimized.

Member

Kevin-Mattheus-Moerman commented Jan 25, 2018

@costashatz thanks for helping out here. I've added a REVIEWER 3 section at the top here. This is where you will work to tick boxes. You should be able to tick them (Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations). Let me know if you have questions.

@Kevin-Mattheus-Moerman

This comment has been minimized.

Member

Kevin-Mattheus-Moerman commented Jan 25, 2018

@betatim I've left your review tickbox set in tact in case you do find time to contribute to the review. Either way thanks for your input.

@Kevin-Mattheus-Moerman

This comment has been minimized.

Member

Kevin-Mattheus-Moerman commented Jan 25, 2018

@costashatz thanks again for jumping in to save the day 🚀 ! Could you give me an indication as to when you expect to finalize the review? Thanks

@costashatz

This comment has been minimized.

Collaborator

costashatz commented Jan 25, 2018

@costashatz thanks again for jumping in to save the day ! Could you give me an indication as to when you expect to finalize the review? Thanks

No problem! Since I am already using the library (i.e., I have it installed, already done the tutorials etc), I think it'll be quick!

@costashatz

This comment has been minimized.

Collaborator

costashatz commented Jan 25, 2018

@jslee02 Great library and submission! Only two small remarks:

  • Is there a specific reason why you do not have API docs for v6.3.0 that is being submitted? I am aware the API didn't change much from v6.1.1 to v6.3.0, but I'd guess it's not difficult to generate it right?
  • Regarding the performance, as I already noted in this issue there seems to be something slowing down the simulation with collisions (mainly the one of FCL) even with simple cases (like the collision/dominoes tutorial). Have you identified what the problem is? Does it come from DART or FCL?

Both of my remarks are minor as the library is very well written and documented!

Thanks!

@jslee02

This comment has been minimized.

jslee02 commented Jan 26, 2018

@costashatz Thank you for the quick feedback. Please find below my answers to your remarks:

  • Updated API docs can be found on the website or API repo.

  • Regarding the collision/contact performance, I replied on the issue:

This is somewhat an expected result. There is two main reasons behind this: (1) collision handling method in DART and (2) number of contact points generated by the collision detector.

(1) DART uses rather slow but more precise contact constraint solver.

AFAIK, there are two major categories of solving non-penetration contact constraint handling methods: (a) LCP-based method and (b) iterative method. I believe DART initially chose (a) method to compute precise contact points sacrificing speed, but we are working on supporting (b) for interactive simulations like manipulation in VR. This is my working branch hoping to finish initial implementation by next couple of months.

(2) FCLCollisionDetector generally returns more contact points than DARTCollisionDetector.

In FCL setting of DART, we use meshes even for primitive shapes (e.g., box and sphere) by default, and this generally results in more contact points than primitive-primitive shape pair contacts (like DARTCollisionDetector). This is because FCL detects all the contact points of the meshes' triangle pairs in contact while primitive-primitive collision algorithm returns minimally required contacts (this can be varied by the concrete primitive collision algorithm). For example, box-box collision algorithm returns up to four contacts.

Loosely speaking, the complexity of (a) method is O(n^3) where n is the number of contacts (assuming no friction). That said, more contacts make the simulate slow quickly.

The reason we don't use FCL's primitive shape support is that it has a few unresolved problems. I had worked on addressing those issues (e.g., flexible-collision-library/fcl#52, flexible-collision-library/fcl#61), but there are some remains.

There is persistent contact manifold technic of Bullet Physics engine that can address this issue. It manages the contact points reported from the collision detector to be less than 4, which is minimal contact points to support the contact shapes each other. I'm also considering having the similar feature in DART.

@costashatz

This comment has been minimized.

Collaborator

costashatz commented Jan 31, 2018

@costashatz Thank you for the quick feedback. Please find below my answers to your remarks:

Updated API docs can be found on the website or API repo.

Regarding the collision/contact performance, I replied on the issue

@jslee02 Thank you for your fast answers (I will continue the discussion in the issue regarding the collision/contact performance). I am very much satisfied by the library and the submission! Best of luck!

@Kevin-Mattheus-Moerman Let me know if you need me to do anything else..

@jslee02

This comment has been minimized.

jslee02 commented Feb 5, 2018

@Kevin-Mattheus-Moerman Sorry if I've made you feel like I'm putting too much pressure on you, but do you think there is anything left needs to be done? It would be great if I could provide the way to cite DART soon to the authors used DART for their research.

@Kevin-Mattheus-Moerman

This comment has been minimized.

Member

Kevin-Mattheus-Moerman commented Feb 5, 2018

@whedon generate pdf

@whedon

This comment has been minimized.

Collaborator

whedon commented Feb 5, 2018

Attempting PDF compilation. Reticulating splines etc...
@whedon

This comment has been minimized.

Collaborator

whedon commented Feb 5, 2018

https://github.com/openjournals/joss-papers/blob/joss.00500/joss.00500/10.21105.joss.00500.pdf
@Kevin-Mattheus-Moerman

This comment has been minimized.

Member

Kevin-Mattheus-Moerman commented Feb 5, 2018

@jslee02 my apologies for the delay in getting back to you! I recommend that this paper is accepted. At this point please provide a DOI for the repository of the final reviewed version. @arfon will then proceed with formal acceptance of this paper. 🚀 🤖 🎉

@Kevin-Mattheus-Moerman

This comment has been minimized.

Member

Kevin-Mattheus-Moerman commented Feb 5, 2018

@betatim @costashatz @bmagyar thanks for reviewing this submission! 👏

@jslee02

This comment has been minimized.

jslee02 commented Feb 5, 2018

@Kevin-Mattheus-Moerman Thank you! I've created a DOI for DART 6.3.0 here:
https://zenodo.org/record/1166142#.WnjLInXwbhw

@Kevin-Mattheus-Moerman

This comment has been minimized.

Member

Kevin-Mattheus-Moerman commented Feb 5, 2018

Great! @arfon we are good to go here 🤖

@arfon arfon added the accepted label Feb 6, 2018

@arfon

This comment has been minimized.

Member

arfon commented Feb 6, 2018

@whedon set 10.5281/zenodo.1166142 as archive

@whedon

This comment has been minimized.

Collaborator

whedon commented Feb 6, 2018

OK. 10.5281/zenodo.1166142 is the archive.

@arfon

This comment has been minimized.

Member

arfon commented Feb 6, 2018

@jslee02 @bmagyar @betatim - many thanks for your reviews here and to @Kevin-Mattheus-Moerman for editing this submission

@jslee02 - your paper is now accepted into JOSS and your DOI is https://doi.org/10.21105/joss.00500 ⚡️ 🚀 💥

@arfon arfon closed this Feb 6, 2018

@whedon

This comment has been minimized.

Collaborator

whedon commented Feb 6, 2018

🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉

If you would like to include a link to your paper from your README use the following code snippet:

[![DOI](http://joss.theoj.org/papers/10.21105/joss.00500/status.svg)](https://doi.org/10.21105/joss.00500)

This is how it will look in your documentation:

DOI

We need your help!

Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider either:

  1. Volunteering to review for us sometime in the future. You can add your name to the reviewer list here: http://joss.theoj.org/reviewer-signup.html
  2. Making a small donation to support our running costs here: https://www.flipcause.com/secure/cause_pdetails/Mjk3Nzk=
@jslee02

This comment has been minimized.

jslee02 commented Feb 6, 2018

Thank you @bmagyar, @betatim, @costashatz for the helpful review; and @Kevin-Mattheus-Moerman, @arfon for coordinating this process! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment