openjournals / joss-reviews

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

[PRE REVIEW]: Open Source Optical Coherence Tomography Software #2245

Closed whedon closed 4 years ago

whedon commented 4 years ago

Submitting author: @spectralcode (Miroslav Zabic) Repository: https://github.com/spectralcode/OCTproZ Version: v1.0.0 Editor: @arfon Reviewers: @phtomlins, @jdavidli Managing EiC: Arfon Smith

: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.

Author instructions

Thanks for submitting your paper to JOSS @spectralcode. Currently, there isn't an JOSS editor assigned to your paper.

@spectralcode if you have any suggestions for potential reviewers then please mention them here in this thread (without tagging them with an @). In addition, this list of people have already agreed to review for JOSS and may be suitable for this submission (please start at the bottom of the list).

Editor instructions

The JOSS submission bot @whedon is here to help you find and assign reviewers and start the main review. To find out what @whedon can do for you type:

@whedon commands
whedon commented 4 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks.

: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.

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

github.com/AlDanial/cloc v 1.84  T=0.50 s (182.4 files/s, 102992.4 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
C++                             36           5385          11190          19692
C/C++ Header                    37           1747           2569           5836
Qt                               4              0              0           2772
CUDA                             1            156            109            941
ProGuard                         4            104             65            412
GLSL                             4             84            112            264
Markdown                         5            116              0            240
TeX                              1             15              0            152
-------------------------------------------------------------------------------
SUM:                            92           7607          14045          30309
-------------------------------------------------------------------------------

Statistical information for the repository '2245' was gathered on 2020/05/23.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
spectralcode                    11         46904            485          100.00

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
spectralcode              46419           99.0          0.0               35.40
whedon commented 4 years ago
Reference check summary:

OK DOIs

- None

MISSING DOIs

- https://doi.org/10.1364/boe.3.003067 may be missing for title: Spectral domain optical coherence tomography of multi-MHz A-scan rates at 1310 nm range and real-time 4D-display up to 41 volumes/second
- https://doi.org/10.1364/oe.18.011772 may be missing for title: Real-time 4D signal processing and visualization using graphics processing unit on a regular nonlinear-k Fourier-domain OCT system
- https://doi.org/10.1109/fccm.2011.27 may be missing for title: Scalable, high performance Fourier domain optical coherence tomography: Why FPGAs and not GPGPUs
- https://doi.org/10.1364/oe.15.007634 may be missing for title: Improved lateral resolution in optical coherence tomography by digital focusing using two-dimensional numerical diffraction method
- https://doi.org/10.1364/boe.8.004887 may be missing for title: Improving lateral resolution and image quality of optical coherence tomography by the multi-frame superresolution technique for 3D tissue imaging
- https://doi.org/10.1117/12.911297 may be missing for title: Digital refocusing in optical coherence tomography
- https://doi.org/10.1117/1.jbo.18.2.026002 may be missing for title: Graphics processing unit accelerated optical coherence tomography processing at megahertz axial scan rate and high resolution video rate volumetric rendering
- https://doi.org/10.1007/978-3-319-06419-2_6 may be missing for title: Spectral/Fourier domain optical coherence tomography
- https://doi.org/10.1364/oe.18.024395 may be missing for title: Reference spectrum extraction and fixed-pattern noise removal in optical coherence tomography
- https://doi.org/10.1364/boe.5.002963 may be missing for title: High definition live 3D-OCT in vivo: design and evaluation of a 4D OCT engine with 1 GVoxel/s

INVALID DOIs

- None
whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

arfon commented 4 years ago

@spectralcode - Thanks for your submission to JOSS. As described in our blog post announcing the reopening of JOSS, we're currently working in a "reduced service mode", limiting the number of papers assigned to any individual editor.

Since reopening JOSS earlier in the week we've had > 50 papers submitted and as such, yours has been put in our backlog that we will be working through over the coming weeks and months.

Thanks in advance for your patience!

spectralcode commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

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

Error reading bibliography ./paper.bib (line 10, column 3): unexpected "d" expecting space, ",", white space or "}" Error running filter pandoc-citeproc: Filter returned error status 1 Looks like we failed to compile the PDF

spectralcode commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

spectralcode commented 4 years ago

@whedon check references

whedon commented 4 years ago
Reference check summary:

OK DOIs

- 10.1364/boe.3.003067 is OK
- 10.1364/oe.18.011772 is OK
- 10.1117/1.3548153 is OK
- 10.1117/1.JBO.17.10.100502 is OK
- 10.1109/fccm.2011.27 is OK
- 10.1364/OPEX.12.002404 is OK
- 10.1364/OPEX.12.002435 is OK
- 10.1364/oe.15.007634 is OK
- 10.1364/BOE.8.004887 is OK
- 10.1117/12.911297 is OK
- 10.1364/OE.11.000782 is OK
- 10.1117/1.JBO.18.2.026002 is OK
- 10.1364/OL.24.001221 is OK
- 10.1007/978-3-540-77550-8_5 is OK
- 10.1364/OE.18.024395 is OK
- 10.1364/BOE.5.002963 is OK
- 10.1371/journal.pone.0213144 is OK

MISSING DOIs

- None

INVALID DOIs

- None
arfon commented 4 years ago

@whedon assign me as editor

whedon commented 4 years ago

OK, the editor is @arfon

arfon commented 4 years ago

:wave: @spectralcode - thanks for your submission to JOSS. I'll be your editor for this submission. A couple of items I could do with your help with:

spectralcode commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

spectralcode commented 4 years ago

@whedon check references

whedon commented 4 years ago
Reference check summary:

OK DOIs

- 10.1364/boe.3.003067 is OK
- 10.1364/oe.18.011772 is OK
- 10.1117/1.3548153 is OK
- 10.1117/1.JBO.17.10.100502 is OK
- 10.1109/fccm.2011.27 is OK
- 10.1117/1.JBO.18.2.026002 is OK
- 10.1364/OE.18.024395 is OK
- 10.1364/BOE.5.002963 is OK

MISSING DOIs

- None

INVALID DOIs

- None
spectralcode commented 4 years ago

@arfon thank you for your comment regarding the paper length. I have shortened the paper drastically. Please let me know if you still have any concerns.

Here are some potential reviewers from the list:

A very suitable reviewer who is not on the list would be datenwolf. However, I don't know if he is willing to review.

arfon commented 4 years ago

:wave: @datenwolf @mstimberg @RainierBarrett - would any of you be willing to review this submission for JOSS? We carry out our checklist-driven reviews here in GitHub issues and follow these guidelines: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html

mstimberg commented 4 years ago

:wave: @arfon. I could review from the technical side, but I have only minimal domain knowledge and don't know other software used in this area. I also have a number of reviews lined up, so this is more of an "ok, if you don't find anyone else" rather than an enthusiastic yes :wink: (the software itself looks pretty impressive though!)

datenwolf commented 4 years ago

I'm okay to review it. However I must stress that any review must clearly indicate my financial interest in Optores. Other than that, I can do it.

arfon commented 4 years ago

I'm okay to review it. However I must stress that any review must clearly indicate my financial interest in Optores. Other than that, I can do it.

OK, thanks for letting me know @datenwolf. I'd rather not avoid this conflict it at all possible.

arfon commented 4 years ago

:wave: @mlxd - would you be willing to review this submission for JOSS? We carry out our checklist-driven reviews here in GitHub issues and follow these guidelines: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html

mlxd commented 4 years ago

Hi @arfon as much as I would like to help, due to current restrictions here I won't have access to capable hardware to evaluate this for another 3 months or so. Apologies, but I will need to pass for now.

If no progress is made by then please feel free to get back to me.

arfon commented 4 years ago

:wave: @adam-m-jcbs - would you be willing to review this submission for JOSS? We carry out our checklist-driven reviews here in GitHub issues and follow these guidelines: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html

RainierBarrett commented 4 years ago

Hi @arfon, I'm afraid I'm gearing up to defend my PhD thesis in the next couple months, so I don't think I'll have time to dedicate to a review for the time being.

arfon commented 4 years ago

NP, thanks for letting us know @RainierBarrett. Also good luck!

arfon commented 4 years ago

@whedon invite @jgostick as editor

:wave: @jgostick - would you be willing to edit this submission? I feel like you might have better luck finding reviewers?

whedon commented 4 years ago

@jgostick has been invited to edit this submission.

jgostick commented 4 years ago

I'm not able to run or compile any of this code, and despite the tomography keyword it's quite different from what I work on, so I don't know anyone that could review. Sorry.

arfon commented 4 years ago

I'm not able to run or compile any of this code, and despite the tomography keyword it's quite different from what I work on, so I don't know anyone that could review. Sorry.

OK thanks for letting me know.

arfon commented 4 years ago

:wave: @marksgraham - would you be willing to review this submission for JOSS? We carry out our checklist-driven reviews here in GitHub issues and follow these guidelines: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html

marksgraham commented 4 years ago

Hi. I've taken a look and am not able to compile the code. Perhaps if @spectralcode can assist me I'd be able to review - I'm on Ubuntu 18.04.

spectralcode commented 4 years ago

Hi @marksgraham I can assist you to compile the code and then I will update the compiling instructions to help future users with similar troubles.

Can you provide some more information? Do you use the QtCreator and do you have installed cuda? Are there any error messages from the compiler?

marksgraham commented 4 years ago

I have installed QTCreator and I have all the CUDA drivers. OCTProZ_DevKit seems to build fine. virtualoctsystem gives three errors when I attempt to build, all related to ceil. Here is an example of one of them:

/home/mark/Downloads/OCTproZ-master/octproz_virtual_oct_system/src/virtualoctsystem.cpp:60: error: ‘ceil’ was not declared in this scope
  size_t bufferSize = currParams.width*currParams.height*currParams.depth*ceil((double)this->currParams.bitDepth / 8.0);
                                                                          ^~~~
spectralcode commented 4 years ago

@marksgraham thank you for taking the time to try to compile this somewhat large project. I'm sure we can get it to work together.

I've updated virtualoctsystem.h (just added the line "#include "math.h""). Could you please retry it with the new virtualoctsystem.h and let me know if it works?

If this does not work another possibility is to replace "ceil" by "std::ceil", but as this problem does not occur with Ubuntu 16.04 I will first have to install Ubuntu 18.04 and test it.

marksgraham commented 4 years ago

@spectralcode I can now compile virtualoctsystem. It isn't clear to me how I actually get it to run now.. when I press run I'm told You need to set an executable in the custom run configuration.

spectralcode commented 4 years ago

@marksgraham ok, thats great! The virtual OCT system is an OCTproZ plugin and it cannot be executed on its own. It needs to be loaded from OCTproZ. Have a look at the quick start guide in the user manual: https://spectralcode.github.io/OCTproZ/index.html

Try compiling OCTproZ and then run it to load the virtual OCT system. (Hint: before running OCTproZ with the "play" button in Qt Creator right click on the OCTproZ project and click on the first option. It should be something like "set as active project")

marksgraham commented 4 years ago

@spectralcode When I now try to compile OCTproZ I get a CUDA error: /bin/sh: 1: /usr/local/cuda/bin/nvcc: not found I had thought I had the relevant drivers because nvidia-smi works on my system with the following output: NVIDIA-SMI 430.50 Driver Version: 430.50 CUDA Version: 10.1

Does this mean I also need to install the nvidia-cuda-toolkit to get this working on my system?

spectralcode commented 4 years ago

@marksgraham yes, exactly.

marksgraham commented 4 years ago

I have nvcc installed at /usr/bin/nvcc but QTcreator is still looking for it in /usr/local/cuda/bin/nvcc. It isn't clear to me how I can configure the build to point it to the right place - any ideas?

arfon commented 4 years ago

@datenwolf - could you say a little more about your financial interests in Optores here? It's not clear what Optores is, and how it's related to this software.

spectralcode commented 4 years ago

@marksgraham I am not sure, but it could be that you need to do the "post-installation actions" which are listed in the official cuda installation guide for Linux.

I am going to set up Ubuntu 18.04 and the Qt + Cuda development environment to see if I run in to the same issue.

spectralcode commented 4 years ago

I've tested it with Ubuntu 18.04 and everything worked just fine. Unfortunately (?) I did not had the same problem as you did with nvcc that can't be found. For installation of cuda I followed the official cuda installation guide for Linux. If you have also used this guide, then probably something went wrong during the post-installation actions as those include the setup of the PATH variable. Do you think this can be the case?

But I've found some other minor issues in the .pro files that are now fixed.

Edit: I updated the compilation section and added instructions for setting up a development environment on Ubuntu. Please let me know if this (or performing the post-installation actions) helped you.

marksgraham commented 4 years ago

Hi @spectralcode @arfon

I've delved deeper and concluded that for me to get this working I would need to do a fresh install of all my CUDA drivers. Experience had taught me that messing with CUDA drivers can cause various problems and should only be done when absolutely necessary. I'm reluctant to do so in order to complete this review, especially as I'm doing a lot of GPU-heavy work right now that would be interrupted if I were to interfere with my setup. So I'm going to have to say sorry, and pass on this.

spectralcode commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

arfon commented 4 years ago

@spectralcode - sorry it's proving so tough to find reviewers here. Any chance you could suggest some additional possible candidates here?

spectralcode commented 4 years ago

@arfon - I still hope that @datenwolf will reply.

Here are some other possible candidates: