search

openjournals / joss-reviews

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

[REVIEW]: A Structured-Light Scanning Software for Rapid Geometry Acquisition #47

Closed whedon closed 6 years ago

whedon commented 8 years ago

Submitting author: @charalambos (Charalambos Poullis) Repository: https://github.com/theICTlab/3DUNDERWORLD-SLS-GPU_CPU Version: v4.0.0 Editor: @Kevin-Mattheus-Moerman Reviewer: @daviddoria Archive: Pending

Status

status

Status badge code:

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

Reviewer questions

Conflict of interest

Paper PDF: 10.21105.joss.00047.pdf

arfon commented 8 years ago

/ cc @openjournals/joss-reviewers - would anyone be willing to review this submission?

If you would like to review this submission then please comment on this thread so that others know you're doing a review (so as not to duplicate effort). Something as simple as :hand: I am reviewing this will suffice.

Reviewer instructions

Any questions, please ask for help by commenting on this issue! 🚀

davclark commented 8 years ago

Does the reviewer need to be a scientist at a non-profit institution? I have a colleague trained as an anthropologist who now runs a 4D scanning company who might be able to review.

arfon commented 8 years ago

Does the reviewer need to be a scientist at a non-profit institution? I have a colleague trained as an anthropologist who now runs a 4D scanning company who might be able to review.

@davclark no, I think your colleague sounds good.

labarba commented 8 years ago

Thanks for jumping in for JOSS, @davclark !!

davclark commented 8 years ago

And thanks for the invite :)

davclark commented 8 years ago

My colleague may or may not have time for this, but he did point out that this paper also has the issue of difficulty of full verification without necessary hardware. So it seems openjournals/joss#156 is relevant here also.

charalambos commented 8 years ago

That is correct in the case where you would like to scan a new object. For testing however, we provide a complete dataset (Alexander folder) which you can use.

Kevin-Mattheus-Moerman commented 8 years ago

If example data is provided and the software is a tool for post-processing this data (i.e. the software and its claims do not rely on or control the hardware) then we can still review this submission. @charalambos for those interested in testing new objects, can you provide a link to/description of the required hardware?

v3c70r commented 8 years ago

To process the example data. The only hardware required is a nVidia graphics card supports CUDA. We used Geforce GTX770 in our development.

charalambos commented 8 years ago

@Kevin-Mattheus-Moerman We haven't completed the detailed users' manual for this version yet, but the procedure for setting up and scanning described in the previous version (http://www.3dunderworld.org/wp-content/uploads/2014/03/3DUNDERWORLD-SLSv3.0.pdf) is the same. Please note that v4 has no dependency on CANON's EOS API

pjotrp commented 8 years ago

How large it the git repo? I interrupted at 100Mb. Usually it is a good idea to have the data separate from the sources.

v3c70r commented 8 years ago

@pjotrp The repository is 299.04MB. I have no problem to clone the repository. But you are right that it is better to have the data separated.

pjotrp commented 8 years ago

OK, put a warning on the size in the README, that should suffice for now.

arfon commented 7 years ago

@whedon commands

whedon commented 7 years ago

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 the GitHub usernames of the JOSS editors
@whedon list editors

# List of JOSS 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

:construction: Important :construction:

This is all quite new. Please make sure you check the top of the issue after running a @whedon command (you might also need to refresh the page to see the issue update).

arfon commented 7 years ago

@whedon list editors

whedon commented 7 years ago

Current JOSS editors:

@acabunoc
@arfon
@cMadan
@danielskatz
@jakevdp
@karthik
@katyhuff
@kyleniemeyer
@labarba
@mgymrek
@pjotrp
@tracykteal
arfon commented 7 years ago

@whedon assign @Kevin-Mattheus-Moerman as editor

whedon commented 7 years ago

OK, the editor is @Kevin-Mattheus-Moerman

Kevin-Mattheus-Moerman commented 7 years ago

Dear authors @charalambos. Apologies for the delay with this submission. I'm in the process of finding reviewers for your submission. If you are able to suggest reviewers please do so as well. Thanks.

charalambos commented 7 years ago

@Kevin-Mattheus-Moerman Thank you for letting us know.

Kevin-Mattheus-Moerman commented 7 years ago

@simonfuhrmann @kangxue @mikhail-matrosov @daeyun @lasvegasrc @daviddoria @krm15 @kysucix is this your cup of tea? Are you willing to sign up as a JOSS reviewer and review this submission?

Link to the submission in JOSS: https://github.com/openjournals/joss-reviews/issues/47

The project GitHub repository: https://github.com/theICTlab/3DUNDERWORLD-SLS-GPU_CPU

The paper: https://github.com/openjournals/joss-reviews/files/402839/10.21105.joss.00047.pdf

The submission's project website: http://www.3dunderworld.org/

Reviewer instructions: http://joss.theoj.org/about#reviewer_guidelines

daviddoria commented 7 years ago

:hand: I will review this weekend.

Kevin-Mattheus-Moerman commented 7 years ago

@charalambos Some editorial comments on your paper. Could all authors share the same affiliation index? (currently affiliations are copied 3 times in paper). In your paper you mention:

We have performed extensive testing with a wide range of models and the results are documented in our report.

Could you expand on this please. This would be very helpful for the current reviewers but also future readers. Where are the models? What sort of models are these? What is the "report", do you mean the documentation? Could you even say something like "The README will guide the user to a suite of test models which allow the user to replicate typical results" (adjust as you see fit)

Kevin-Mattheus-Moerman commented 7 years ago

@daviddoria Thanks! Welcome on board Sir!

Kevin-Mattheus-Moerman commented 7 years ago

@whedon assign @daviddoria as reviewer

whedon commented 7 years ago

I'm sorry @Kevin-Mattheus-Moerman, I'm afraid I can't do that. That's something only JOSS editors are allowed to do.

Kevin-Mattheus-Moerman commented 7 years ago

@arfon Can you check what is wrong here?

Kevin-Mattheus-Moerman commented 7 years ago

@rougier do you know an additional reviewer at INRIA perhaps?

arfon commented 7 years ago

@whedon list editors

whedon commented 7 years ago

Current JOSS editors:

@acabunoc
@arfon
@biorelated
@cMadan
@danielskatz
@jakevdp
@karthik
@katyhuff
@Kevin-Mattheus-Moerman
@kyleniemeyer
@labarba
@mgymrek
@pjotrp
@tracykteal
arfon commented 7 years ago

@Kevin-Mattheus-Moerman you should be good to go now.

Kevin-Mattheus-Moerman commented 7 years ago

@whedon assign @daviddoria as reviewer

arfon commented 7 years ago

@whedon assign @daviddoria as reviewer

arfon commented 7 years ago

@whedon assign @daviddoria as reviewer

whedon commented 7 years ago

OK, the reviewer is @daviddoria

daviddoria commented 7 years ago

All,

Please excuse the "newbie" questions as I'm new to this process :).

I started checking boxes at the top of this issue for things that I verified. Presumably for things that I do NOT check there should be somewhere to explain why I did not check them?

I am having trouble building the project. I have listed the first two issues below - is this where I should discuss these things? Can/should I contact the author directly to get help like this and then ensure the documentation is updated?

1) This error: /home/doria/source/3DUNDERWORLD-SLS-GPU_CPU/src/lib/GrayCode/GrayCode.hpp:2:30: fatal error: opencv2/opencv.hpp: No such file or directory

include <opencv2/opencv.hpp>

seems resolvable by adding a cmake line: INCLUDE_DIRECTORIES(${OpenCV_INCLUDE_DIRS})

2) I then get this error:

In file included from /home/doria/source/3DUNDERWORLD-SLS-GPU_CPU/./src/lib/core/Camera.h:24:0, from /home/doria/source/3DUNDERWORLD-SLS-GPU_CPU/./src/lib/core/fileReader.h:19, from /home/doria/source/3DUNDERWORLD-SLS-GPU_CPU/./src/lib/calibration/Calibrator.hpp:2, from /home/doria/source/3DUNDERWORLD-SLS-GPU_CPU/src/lib/calibration/Calibrator.cpp:1: /home/doria/source/3DUNDERWORLD-SLS-GPU_CPU/./src/lib/core/Dynamic_Bitset.h:29:23: fatal error: glm/glm.hpp: No such file or directory

include <glm/glm.hpp>

I tried apt-get install glm-dev but that does not seem to be a package (Ubuntu 14.04).

Thanks,

David

daviddoria commented 7 years ago

@Kevin-Mattheus-Moerman FYI the link you posted in your post "is this your cup of tea? Are you willing to sign up as a JOSS reviewer and review this submission?" links to https://github.com/openjournals/joss-reviews/issues/joss.theoj.org which seems to be broken.

daviddoria commented 7 years ago

@charalambos The link to the paper https://github.com/openjournals/joss-reviews/files/402839/10.21105.joss.00047.pdf is an extremely short document with no images, etc. However I see in your /doc folder that there are a bunch of image files. Is there a more "real" document that I'm missing?

v3c70r commented 7 years ago

@daviddoria GLM can be installed with sudo apt install libglm-dev on Ubuntu.

daviddoria commented 7 years ago

This does not build against OpenCV3, which is fine, but it just needs to indicate the versions of the dependencies somewhere.

daviddoria commented 7 years ago

I saw this in Line 14 of Calibrator.hpp:

static std::condition_variable cv;

That seems really fragile given that there is the 'cv' namespace for OpenCV.

daviddoria commented 7 years ago

It seems as though an "in source" build is expected (which is almost never recommended). I say that because: 1) The readme says "Binaries are compiled and located in the bin folder." which presumably is implying [sourceTree]/bin.

2) When trying to run the executable (I cloned in ~/source/underworld and built in ~/build/underworld), I get:

doria@doria-home:~/build/underworld$ ./bin/SLS No image read from ../../data/alexander/rightCam/dataset1/ ... No image read from ../../data/alexander/leftCam/dataset1/ ... Failed to load config ../../data/alexander/rightCam/calib/output/calib.xml Failed to load config ../../data/alexander/rightCam/calib/output/calib.xml Failed to load config ../../data/alexander/rightCam/calib/output/calib.xml Failed to load config ../../data/alexander/rightCam/calib/output/calib.xml Segmentation fault (core dumped)

which seems to be referencing files relative to the build tree (which is expected to be the source tree). This is bad - these paths should be specifiable on the command line.

Kevin-Mattheus-Moerman commented 7 years ago

@daviddoria thanks again for reviewing this submission. To answer your questions.

I started checking boxes at the top of this issue for things that I verified. Presumably for things that I do NOT check there should be somewhere to explain why I did not check them? I am having trouble building the project. I have listed the first two issues below - is this where I should discuss these things? Can/should I contact the author directly to get help like this and then ensure the documentation is updated?

Yes, please post any comments and issues you want to communicate to the authors here. This way the entire review process is open and traceable.

Kevin-Mattheus-Moerman commented 7 years ago

@daviddoria in relation to

@charalambos The link to the paper https://github.com/openjournals/joss-reviews/files/402839/10.21105.joss.00047.pdf is an extremely short document with no images, etc. However I see in your /doc folder that there are a bunch of image files. Is there a more "real" document that I'm missing?

Papers in JOSS are deliberately short. See some of our motivation behind it here: http://www.arfon.org/announcing-the-journal-of-open-source-software

Here are are paper content requirements: http://joss.theoj.org/about where it says

JOSS papers should contain the following:

  • A list of the authors of the software
  • Author affiliations
  • A short summary describing the high-level functionality of the software
  • A list of key references including a link to the software archive

and see some examples of accepted papers here: http://joss.theoj.org/papers/popular

So the "real" document you refer to (you probably mean a classic full length paper) is not required if the software documentation is complete enough. So if you find the descriptions are lacking feel free to also demand expanded documentation.

Kevin-Mattheus-Moerman commented 7 years ago

@daviddoria authors are allowed to include figures in the paper however so of course you can also recommend that if you feel it is needed.

charalambos commented 7 years ago

@daviddoria There is also a "traditional" paper that goes along with the code and described the technique at arxiv: http://arxiv.org/abs/1406.6595

There is also a user's manual for setting up and using the software. This was written for a previous version but the setup is identical: http://www.3dunderworld.org/wp-content/uploads/2014/03/3DUNDERWORLD-SLSv3.0.pdf

daviddoria commented 7 years ago

@charalambos Thanks for the link to the traditional paper. The setup instructions still do not seem to list anything about prerequisites (which version of cmake/opencv/etc is required, which packages likely need to be installed on common platforms (the GLM issue I had), etc.

Did you see my comment about the assumption about an in-source build? I was going to say that is blocking me from continuing, but suppose I could do an in-source build to continue the review for now... haha

@Kevin-Mattheus-Moerman - is it appropriate to put a link to the arXiv paper in the JOSS paper? It's really hard to start a review without knowing what the code is even supposed to be implementing.

v3c70r commented 7 years ago

@daviddoria Yes, I saw your comment on the code. Sorry I didn't get chance to touch the code last weekend, I'll fix it later tonight.

Kevin-Mattheus-Moerman commented 7 years ago

@charalambos @daviddoria the paper may cite other and previous works. However the software submission for JOSS should stand-alone i.e. we cannot simply refer to another paper and old documentation from a previous version.

I would prefer if the authors did the following. 1) Expand the paper to give a more complete description of the software functionality. Consider including an overview figure to graphically show the software capabilities. The paper can remain short but should give readers and reviewers enough information to understand the capabilities of the software. As long as the JOSS paper provides a sufficient "stand-alone" description, feel free to also refer to previous work/papers. 2) Please fix the documentation based on the reviewers comments. Also please update the documentation for the version submitted here. If the documentation for the previous version is indeed very similar it should be little work to update.