search

openjournals / joss-reviews

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

[REVIEW]: CusToM: a Matlab toolbox for musculoskeletal simulation #927

Closed whedon closed 5 years ago

whedon commented 6 years ago

Submitting author: @anmuller (Antoine Muller) Repository: https://github.com/anmuller/CusToM.git Version: v1.1.0 Editor: @Kevin-Mattheus-Moerman Reviewers: @modenaxe, @demotu, @nicoguaro Archive: 10.5281/zenodo.2543645

Status

status

Status badge code:

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

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

@modenaxe & @demotu, 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.

✨ Please try and complete your review in the next two weeks ✨

Review checklist for @modenaxe

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @demotu

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @nicoguaro

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 6 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @modenaxe, it looks like you're currently assigned as the reviewer for this paper :tada:.

: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
whedon commented 6 years ago 
Attempting PDF compilation. Reticulating splines etc...
whedon commented 6 years ago

--> Check article proof :page_facing_up: <--

Kevin-Mattheus-Moerman commented 6 years ago

@modenaxe, @demotu this is where the review will take place. You each have a set of check boxes at the top of this issue to guide you through the review process. Please let me know if you have any questions or check the review guidelines. Thanks! :rocket: :robot:

Kevin-Mattheus-Moerman commented 6 years ago

@demotu We will get started here already but it is fine if you join the review process in October. Thanks.

Kevin-Mattheus-Moerman commented 6 years ago

@anmuller this is where the review process takes place so please keep an eye out for notifications from this issue. Here are some initial comments/questions I have:

  1. Is this submission compatible with Octave?
  2. Can you please work on adding community guidelines? I recommend adding a CONTRIBUTING.md and a CODE_OF_CONDUCT.md file. https://help.github.com/articles/setting-guidelines-for-repository-contributors/ You can get a template for the code of conduct file from here: https://www.contributor-covenant.org/version/1/4/code-of-conduct.html E.g. check out how my personal project has these files: https://github.com/gibbonCode/GIBBON
anmuller commented 6 years ago

@Kevin-Mattheus-Moerman, I am sorry but it is not compatible with Octave. Thank you for your recommendations about community guidelines, I am going to work it.

modenaxe commented 6 years ago

I just started checking the paper and the material. It's the first time I review for JOSS, so please be patient with me @Kevin-Mattheus-Moerman if I involve you at times :)

General checks

Functionality I installed the toolbox has described in the documentation (just adding the 'Function' folder to the path and ensuring I have all the toolboxes). Then I run 'GenerateParameters.mlapp': the GUI appears but any attempt to use it generates errors. I will follow up in the toolbox repository opening an issue.

Documentation

Software paper Just few comments:

anmuller commented 6 years ago

@modenaxe thank you for your review. What are the errors that appeared when you run 'GenerateParameters.mlapp'?

modenaxe commented 6 years ago

I have posted them as an issue in the repository, let me know if you want more details.

Kevin-Mattheus-Moerman commented 6 years ago

@nicoguaro would you be interested in joining this review process? I can add you as a reviewer if so.

Kevin-Mattheus-Moerman commented 6 years ago

@anmuller can you give an update as to how you are getting on in terms of replying to the issues raised?

Kevin-Mattheus-Moerman commented 6 years ago

@demotu how are you getting on? Could you give an indication as to when you will complete the review process? Thanks.

modenaxe commented 6 years ago

I am updating my comments after testing the revised toolbox. @anmuller my comments follow exactly the guidelines and checkboxes above. @Kevin-Mattheus-Moerman this is currently my complete review, excluding the issues that I will post in the repository directly.

General checks

Functionality I find challenging to test the functionalities using my own data, mainly due to the documentation, which consists in descriptions of the tabs, but does not help getting started with the toolbox. The included examples do not help much either, as loading the mat file for the analysis and visualisation steps seems to throw errors (which I will report as issues in the repository).

Documentation

Software paper Just few comments:

nicoguaro commented 6 years ago

@Kevin-Mattheus-Moerman, I think I can. I have a question, though, is MATLAB necessary for the software or can it be used with Octave? If MATLAB is required, what is the minimum version needed?

Also, from just checking the README in the repo I can't find installation instructions for the package. Or a simple example for "starters". I also suggest to format the README to take advantage of Markdown.

Kevin-Mattheus-Moerman commented 6 years ago

@whedon add @nicoguaro as reviewer

whedon commented 6 years ago

OK, @nicoguaro is now a reviewer

Kevin-Mattheus-Moerman commented 6 years ago

@nicoguaro there is now also a checklist for you at the top of this issue. Yes I believe MATLAB is required. @anmuller can you comment on what version is required?

anmuller commented 6 years ago

It was implemented and tested with Matlab2018a. Also the Symbolic Math, the Optimization and the Parallel Computing toolboxes are necessary

nicoguaro commented 6 years ago

@anmuller I have access to Matlab2017a, is that OK.

I can't find installation instructions and a test script to check that things work.

anmuller commented 6 years ago

@nicoguaro Maybe you can use Matlab2017a but I can't ensure you it works. The installation only consists in adding the folder \CusToM\Functions and its subfolders to the Matlab path. You can check your Matlab version by just running the function "Main" having previously set your current folder as Examples/Cycling or Examples/ROM. If you are interesting, we created a very quick example of the use of an exemple : https://drive.google.com/file/d/1_zpWBwQ-S9pBaHbCBJj0qKMDQoVt02PC/view In the video, all the CusToM folders were added in the path but you have just to add the Functions folders as detailed previously.

nicoguaro commented 6 years ago

@anmuller, I consider that both the installation instructions and example should be in the repo. Hopefully, in the README.

modenaxe commented 6 years ago

I agree with @nicoguaro.

anmuller commented 5 years ago

Thank you for your comments. Installation instructions and instructions for the examples were added in the ReadMe. The two examples were retested and everything works. Please note that for the example "ROM", no measured external forces are available, the ground reaction forces used for the inverse dynamics step are estimated with a prediction method.

nicoguaro commented 5 years ago

I followed the instructions provided in the repo to run Main.m and get the following result

Error using load
Unable to read file 'ModelParameters.mat'. No such file or directory.
cpontonn commented 5 years ago

Dear nicoguaro, did you made the working folder of matlab the one of the example you wanted to run ? If there is no ModelParameters.mat in the folder, therefore you need to create it thanks to the GenerateParameters GUI.

By the way, I am Charles Pontonnier, one of the co-authors of this toolbox. In view of the numerous issues the reviewers raised during the review, we are preparing a lot of additional content to make the handling of the toolbox easier to realize. It may take a bit of time, but we hope that we can commit an amended version of the code and the doc before the end of next week.

nicoguaro commented 5 years ago

@cpontonn, I will wait those changes before any other attempt then.

anmuller commented 5 years ago

Thanks for your comments. This is also our first time here and we are sorry if we are not that efficient in answering to everything quickly. Please find the answers of your comments/recommandations. Also a new version of CusToM was committed.

Reviewer 1 : @Kevin-Mattheus-Moerman

Can you please work on adding community guidelines? I recommend adding a CONTRIBUTING.md and a CODE_OF_CONDUCT.md file. https://help.github.com/articles/setting-guidelines-for-repository-contributors/ You can get a template for the code of conduct file from here : https://www.contributor-covenant.org/version/1/4/code-of-conduct.html E.g. check out how my personal project has these files: https://github.com/gibbonCode/GIBBON Answer: We followed your recommandations and we added, the two .md files: CODE_OF_CONDUCT.md and CONTRIBUTING.md.

Reviewer 2 : @modenaxe

The repository has no history and the functions have no author(s) in the header. Answer: It has been fixed (authoring in the headers notably).

The documentation is a bit vague about how to load c3d files i.e. apparently they are read if 'in the current folder'. Is that Matlab current working folder? If I copy c3d files on that, and then launch the Analyse GUI, it doesn't seem to read them though, and there is no other obvious way of specifying a folder. I have tried few options and even wondered if it might be due to a lag in the GUI, as in my machine I have noticed it's a bit slow at updating. Answer: Normally the way to proceed is to make your analysis folder the working folder of matlab. The Analyse GUI is likely to read them from here.

Could you double check if you can run the provided examples without errors? When I start the provided GUIs, load the parameters from the examples folder and then press the 'run' button, Matlab is always complaining and not running the analyses or the visualization. I can post a more systematic description of the errors if you ensure that the examples are functional in the first place. Answer: Examples are working to the best of our knowledge: all of the authors checked separately their execution and it worked well for all. We noticed a few cases in which you may have errors: in example 2, if you consider reading external forces from C3D data, it may throw and error since such data is not existing in the C3D. We consider adding an example in which external forces are included in the C3D file to avoid such mistakes. In view of your other comments, we consider writing step-by-step examples with accompanying videos to make it easier to run.

Documentation: please mention who your target audience is. Answer: It has been done in the doc. We are targeting students and researchers in motion analysis, as well as anyone interested in motion analysis (trainers, ergonomists,...)

please list explicitly the dependencies of the toolbox. Currently, in the 'installation instructions' section, it is unclear if the additional Matlab Toolboxes are required for having CusToM working or working 'efficiently', e.g. faster processing. Answer: The dependencies are mandatory, indeed. We clarified in the doc.

the title of section 3 should be modified from 'Application Programming Interface' to 'Graphical User Interface'. Answer: Ok, modified I think you should specify in the guide that 'GenerateParameters' is in the 'Interface' folder (I assume that is the file you suggest to run in the guide). I will comment on the rest of the guide after I manage to have the toolbox running. Answer: Ok, modified In almost all occasions in which you are referring 'saving/running/modifying a file in the corresponding folder' I think you should specify to which file and folder you are referring to, so helping the users. I will post a related issue about reading c3ds in the repository. Answer: Ok, modified. as mentioned above the documentation is descriptive but does not include a real step-by-step example or automated tests to verify the functionality of the code. i think a step-by-step example is crucial here. Answer: we agree, and we modified. Community guidelines for contributions/reporting issues/seeking support are currently missing. Answer: we handled that the way @Kevin-Mattheus-Moerman proposed.

Software paper: missing comma after 'body parts'. 'set of muscles': I would add a reference to the anatomical datasets you implemented here. 'thanks to' I would mention explicitly BTK before citing the paper. there is a missing reference just after 'inverse dynamics step'. 'Antoine Muller et al. 2017': Please make citation style consistent throughout the manuscript. Figure 1 is not referenced in the paper and it would be more clear if it was specified explicitly what the external inputs of the boxes are, as in the similar Figure that you have in Muller, Hearing et al. 2017. Answer: We fixed the issues in the paper.

Reviewer 3 : @nicoguaro

From just checking the README in the repo I can't find installation instructions for the package. Or a simple example for "starters". I also suggest to format the README to take advantage of Markdown. Answer: It has been modified.

I consider that both the installation instructions and example should be in the repo. Hopefully, in the README. Answer: It has been modified.

Kevin-Mattheus-Moerman commented 5 years ago

@demotu are you able to join the review process at this stage? Thanks!

Kevin-Mattheus-Moerman commented 5 years ago

@anmuller, @modenaxe, @demotu, @nicoguaro, can you provide an update on where we stand in the review process? Did @anmuller respond to your issues appropriately? What are the remaining issues? Thanks!

cpontonn commented 5 years ago

@modenaxe , @nicoguaro , @demotu , can we help in any way to advance the review process ? Thanks in advance !

modenaxe commented 5 years ago

@Kevin-Mattheus-Moerman @cpontonn I will review the updated repository on Monday.

modenaxe commented 5 years ago

@anmuller thank you for replying to my comments. I have tested the updated toolbox. Few points: 1) you still don't have a version for your current toolbox. I would just tag the current version using git. 2) the toolbox seems to work also with MATLABR2017b, is there a reason why the 'Installation.m' file requires R2018a? 3) the toolbox works, but when I run the analyses from the GUI, MATLAB does not appear as 'Busy' and it takes several minutes for the computations. Maybe this is due to the Matlab version? (the use of computational resources, i.e. CPU and memory is low). For the rest all seems to work, well done! If you address this last comments for me the paper is accepted.

anmuller commented 5 years ago

@modenaxe Thank you for your review and for the different comments.

Please find the answers of the points you mentionned:

  1. We tagged the current version of the toolbox.
  2. Indeed, the toolbox seems to work with Matlab2017b but as the development was done with the 2018a version, we can not guarantee that the code could be run on previous versions. Also, the graphical use interfaces could only be modified on Matlab2018a.
  3. It is normal that the computation takes several minutes (the generation of the biomechanical model and the different motion analysis steps may take some time). However, we do not know why Matlab does not appear 'Busy' during these computations.

We hope this has adressed your points.

modenaxe commented 5 years ago

@anmuller Thank you, for me the paper can be accepted. Well done!

Kevin-Mattheus-Moerman commented 5 years ago

@modenaxe Thanks for reviewing this for JOSS! :tada:

Kevin-Mattheus-Moerman commented 5 years ago

@nicoguaro @demotu are you able to resume and finalize the review at this point?

demotu commented 5 years ago

I will do it in a few hours.

Marcos

On Fri, Dec 14, 2018 at 1:30 PM Kevin Mattheus Moerman < notifications@github.com> wrote:

@nicoguaro https://github.com/nicoguaro @demotu https://github.com/demotu are you able to resume and finalize the review at this point?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/927#issuecomment-447359334, or mute the thread https://github.com/notifications/unsubscribe-auth/ADHrnezjxV5EpdlEOV-UTPatOeSfx3spks5u48QOgaJpZM4WYlaE .

demotu commented 5 years ago

I tested CusToM using matlab 2018b in a windows machine. With the help of the documentation and the tutorials on youtube, i was able to run the three examples that come with the software. This software is for a very specialized public but it works as described. I think after all the improvements the authors have made this paper is ready to be accepted.

nicoguaro commented 5 years ago

@Kevin-Mattheus-Moerman, I tried to run the example described in section 4.1 of the documentation in the following Linux system:

CentOS Linux release 7.4.1708

and obtained the following error:

MathWorks::System::IUserException [Error using matlab.internal.webwindow (line 312)
The MATLABWindow application failed to launch because of missing libraries. Install the packages which contain the following libraries:

    libXss.so.1

Other details

Other minor details are:

Kevin-Mattheus-Moerman commented 5 years ago

@nicoguaro thanks for these comments. @anmuller can you work on these points?

Kevin-Mattheus-Moerman commented 5 years ago

The version number can be set/fixed properly at a later stage (e.g. we assign the correct one if the paper is accepted).

Ipuch commented 5 years ago

Thank you @nicoguaro.

Unfortunately, the library have been developped only on Windows 10. We added the information in the documentation and in the README.md.

obtained the following error: MathWorks::System::IUserExceptionError using matlab.internal.webwindow (line 312) The MATLABWindow application failed to launch because of missing libraries. Install the packages which contain the following libraries: libXss.so.1

I found two hypotheses to your issue :

Here is the statement of need we added to the README.md before the summary and to the documentation:

Inverse dynamics based musculoskeletal analysis aims at calculating biomechanical quantities to understand motion from joint kinematics to muscle forces. Generic models generally based on cadaveric templates are used as an input. These generic models are based on three layers modelisation. The first one is geometrical defining the polyarticulated rigid body system and the kinematic joints. The inertial layer defines mass, centers of mass position and inertia matrix of rigid body of the polyarticulated system. And, the muscle layer defines the muscle paths and force generation behaviours of muscles. This generic models are then calibrated through multiple calibration steps to be subject-specific base on motion capture data. Finally the subject-specific model is used to understand recorded motions of the subject.

However, musculoskeletal simulation requires high computational cost. Editing and assembling features to modify generic models. Subject-specific calibrations and multiple simulations are required to compute subject-specific quantities on recorded trials. And current available musculoskeletal softwares are heavy and requires expertise like SIMM (MusculoGraphics, Inc., Santa Rosa, CA), Anybody (Anybody Technology, Aalborg, Denmark) and OpenSim OpenSim (Simtk, Stanford, CA). Moreover, SIMM and Anybody are commercial softwares and, OpenSim is a freely available software but main algorithms source codes are not available. That's why, there was a need in developping a complete open-source software for musculoskeletal simulation. The source code was developped with Matlab to allow many researchers to understand and contribute to the code.

References of the summary in the README.md were added to a bibliography at the end of the document. The formatting of the citations were revised.

The version number will be set later stage as recommended by @Kevin-Mattheus-Moerman

Kevin-Mattheus-Moerman commented 5 years ago

@nicoguaro are you happy with the reply to your comments @Ipuch formulated?

nicoguaro commented 5 years ago

@Kevin-Mattheus-Moerman, currently I don't have access to MATLAB 2018 under Windows. So I can't attest for the functionality and performance. Besides that I am content with how they addressed my comments.

cpontonn commented 5 years ago

Thank you for your comments. @Kevin-Mattheus-Moerman , since we do not consider developing a Linux compatible version for now, what is the next step to validate the review ? Thanks in advance.

Kevin-Mattheus-Moerman commented 5 years ago

@nicoguaro thanks. I understand. Are you able to tick the boxes for the paper and the documentation?

Kevin-Mattheus-Moerman commented 5 years ago

@cpontonn agreed. I'll wait to see if @nicoguaro can comment on the paper etc. and we will proceed from there.

Kevin-Mattheus-Moerman commented 5 years ago

@whedon generate pdf

whedon commented 5 years ago 
Attempting PDF compilation. Reticulating splines etc...