search

openjournals / joss-reviews

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

[PRE REVIEW]: PIVC: A C/C++ Program for Particle Image Velocimetry Vector Computation #3603

Closed whedon closed 3 years ago

whedon commented 3 years ago

Submitting author: @fibreglass2 (Kadeem) Repository: https://gitlab.com/fibreglass/pivc Version: v1.0.0 Editor: @Kevin-Mattheus-Moerman Reviewers: @timdewhirst, @clarka34, @quynhneo 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.

Status

status

Status badge code:

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

Author instructions

Thanks for submitting your paper to JOSS @https://gitlab.com/fibreglass. Currently, there isn't an JOSS editor assigned to your paper.

@https://gitlab.com/fibreglass 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 3 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 3 years ago

Wordcount for paper.md is 946

whedon commented 3 years ago 
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.03 s (514.8 files/s, 60230.3 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
C++                              4             72             78            794
TeX                              1             33              0            245
Markdown                         3             47              0            169
C/C++ Header                     4             15              1             59
make                             1              1              0              7
-------------------------------------------------------------------------------
SUM:                            13            168             79           1274
-------------------------------------------------------------------------------

Statistical information for the repository 'f1d7693b5c2b55677911fe74' was
gathered on 2021/08/11.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
fibreglass                      14          1387            368          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
fibreglass                 1019           73.5          0.0                7.75
whedon commented 3 years ago

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

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

OK DOIs

- 10.5334/jors.334 is OK
- 10.5334/jors.bl is OK

MISSING DOIs

- 10.1007/s003489900085 may be a valid DOI for title: Comparison of Gaussian particle center estimators and the achievable measurement density for particle tracking velocimetry
- 10.1088/0169-5983/47/3/035509 may be a valid DOI for title: The characteristics of coherent structures in low Reynolds number mixed convection flows
- 10.1016/j.ijthermalsci.2013.11.001 may be a valid DOI for title: The influence of bottom wall heating on the mean and turbulent flow behavior in the near wall region during mixed convection
- 10.1063/1.1375144 may be a valid DOI for title: Simultaneous particle image velocimetry and infrared imagery of microscale breaking waves
- 10.1016/j.expthermflusci.2020.110286 may be a valid DOI for title: An experimental study of the aerodynamics of micro corrugated wings at low Reynolds number
- 10.1016/j.est.2019.100825 may be a valid DOI for title: Investigation of the influence of heat source orientation on the transient flow behavior during PCM melting using particle image velocimetry
- 10.1016/j.ijheatfluidflow.2021.108839 may be a valid DOI for title: The influence of wall heating on turbulent boundary layer characteristics during mixed convection
- 10.1016/j.ijheatfluidflow.2008.01.003 may be a valid DOI for title: The influence of wall heating on the flow structure in the near-wall region
- 10.1063/1.2185709 may be a valid DOI for title: Turbulent Structure beneath Air-Water Interface during Natural Convection
- 10.1007/s00231-005-0072-8 may be a valid DOI for title: Characteristics of Air and Water Velocity Fields during Natural Convection
- 10.1063/1.3054153 may be a valid DOI for title: An Experimental Study of the Airside Flow Structure during Natural Convection
- 10.1088/0957-0233/18/1/012 may be a valid DOI for title: Velocity measurements around a freely swimming fish using PIV
- 10.1017/s0022112006003892 may be a valid DOI for title: Characteristics of the Wind Drift Layer and Microscale Breaking Waves
- 10.1016/j.ijthermalsci.2015.05.003 may be a valid DOI for title: Investigation of fundamental flow mechanisms over a corrugated waveform using proper orthogonal decomposition and spectral analyses
- 10.1016/j.jweia.2021.104605 may be a valid DOI for title: Turbulent flow characterization near the edge of a steep escarpment
- 10.1002/we.1895 may be a valid DOI for title: Flow Characterization in the near-wake region of a horizontal axis wind turbine
- 10.1007/s10236-008-0132-y may be a valid DOI for title: Airside velocity measurements over the wind-sheared water surface using Particle Image Velocimetry

INVALID DOIs

- None
arfon commented 3 years ago

Pausing as we don't have a GitHub handle for the author (I've emailed them about this).

Also, querying scope due to size.

arfon commented 3 years ago

@whedon query scope

whedon commented 3 years ago

Submission flagged for editorial review.

fibreglass2 commented 3 years ago

Pausing as we don't have a GitHub handle for the author (I've emailed them about this).

This is my Github account.

Also, querying scope due to size.

Take caution when observing the current line of code count. This program was originally much larger in terms of LoC, total files, and written to utilize a proprietary closed-source API. My coauthors and I started this work in 2017 to optimize the original code as much as possible, hence greatly reducing the total files and LoC present, obtain gains in computation speed, and eliminate the requirement of a proprietary closed-source API.

Kevin-Mattheus-Moerman commented 3 years ago

@fibreglass2 thanks for this submission. I quickly scanned through your paper as part of the scope review. Some quick comments:

If you make any changes to the paper or bib file you can call the following in a comment to update the paper: @whedon generate pdf and to to check references: @whedon check references

kyleniemeyer commented 3 years ago

@fibreglass2 we're going to go ahead and proceed with review on this one—can you please address the comments that @Kevin-Mattheus-Moerman raised?

Kevin-Mattheus-Moerman commented 3 years ago

@kyleniemeyer FYI I can edit this one if you like

kyleniemeyer commented 3 years ago

@whedon assign @Kevin-Mattheus-Moerman as editor

@Kevin-Mattheus-Moerman sure thing! Wasn't going to ask but happy to accept the offer.

whedon commented 3 years ago

OK, the editor is @Kevin-Mattheus-Moerman

fibreglass2 commented 3 years ago

@fibreglass2 we're going to go ahead and proceed with review on this one—can you please address the comments that @Kevin-Mattheus-Moerman raised?

Certainly. Expect to see updates soon.

fibreglass2 commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

fibreglass2 commented 3 years ago

@whedon check references

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

OK DOIs

- 10.1007/s003489900085 is OK
- 10.5334/jors.334 is OK
- 10.5334/jors.bl is OK
- 10.1088/0169-5983/47/3/035509 is OK
- 10.1016/j.ijthermalsci.2013.11.001 is OK
- 10.1063/1.1375144 is OK
- 10.1016/j.expthermflusci.2020.110286 is OK
- 10.1016/j.est.2019.100825 is OK
- 10.1016/j.ijheatfluidflow.2021.108839 is OK
- 10.1016/j.ijheatfluidflow.2008.01.003 is OK
- 10.1063/1.2185709 is OK
- 10.1007/s00231-005-0072-8 is OK
- 10.1063/1.3054153 is OK
- 10.1088/0957-0233/18/1/012 is OK
- 10.1017/s0022112006003892 is OK
- 10.1016/j.ijthermalsci.2015.05.003 is OK
- 10.1016/j.jweia.2021.104605 is OK
- 10.1002/we.1895 is OK
- 10.1007/s10236-008-0132-y is OK

MISSING DOIs

- 10.1007/s00348-016-2173-1 may be a valid DOI for title: Main results of the 4th International PIV Challenge
- 10.1016/j.ijheatfluidflow.2013.12.005 may be a valid DOI for title: The effect of mixed convection on the structure of channel flow at low Reynolds numbers
- 10.1016/j.ijthermalsci.2014.11.026 may be a valid DOI for title: Flow development during low Reynolds number mixed convection

INVALID DOIs

- None
fibreglass2 commented 3 years ago

@whedon check references

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

OK DOIs

- 10.1007/s003489900085 is OK
- 10.1007/s00348-016-2173-1 is OK
- 10.5334/jors.334 is OK
- 10.5334/jors.bl is OK
- 10.1088/0169-5983/47/3/035509 is OK
- 10.1016/j.ijthermalsci.2013.11.001 is OK
- 10.1016/j.ijheatfluidflow.2013.12.005 is OK
- 10.1016/j.ijthermalsci.2014.11.026 is OK
- 10.1063/1.1375144 is OK
- 10.1016/j.expthermflusci.2020.110286 is OK
- 10.1016/j.est.2019.100825 is OK
- 10.1016/j.ijheatfluidflow.2021.108839 is OK
- 10.1016/j.ijheatfluidflow.2008.01.003 is OK
- 10.1063/1.2185709 is OK
- 10.1007/s00231-005-0072-8 is OK
- 10.1063/1.3054153 is OK
- 10.1088/0957-0233/18/1/012 is OK
- 10.1017/s0022112006003892 is OK
- 10.1016/j.ijthermalsci.2015.05.003 is OK
- 10.1016/j.jweia.2021.104605 is OK
- 10.1002/we.1895 is OK
- 10.1007/s10236-008-0132-y is OK
- 10.1016/j.solener.2010.02.008 is OK

MISSING DOIs

- None

INVALID DOIs

- None
fibreglass2 commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

fibreglass2 commented 3 years ago

@Kevin-Mattheus-Moerman

Kevin-Mattheus-Moerman commented 3 years ago

@alexlib @yosefm @eguvep @clarka34 @andreall @dcsale would you be interested in reviewing this work for JOSS?

The review focuses on the software itself, as well as a short paper. The streamlined review process takes place on GitHub (here is an example of an ongoing review: https://github.com/openjournals/joss-reviews/issues/3611#issuecomment-915658561).

Let me know if you are interested. Note we can be flexible regarding time taken to review.

alexlib commented 3 years ago

@alexlib @yosefm @eguvep @clarka34 @andreall @dcsale would you be interested in reviewing this work for JOSS?

The review focuses on the software itself, as well as a short paper. The streamlined review process takes place on GitHub (here is an example of an ongoing review: #3611 (comment)).

Let me know if you are interested. Note we can be flexible regarding time taken to review.

@Kevin-Mattheus-Moerman thanks but I could not review it properly. the compilation and infrastructure needed to check the claims of OpenMP are beyond my everyday skills. I suggest to ask @paugier or @timdewhirst

andreall commented 3 years ago

@Kevin-Mattheus-Moerman thanks but I don't think that I could review it properly as the software and compilation exceed my expertise. Regards,

timdewhirst commented 3 years ago

Happy to review the code - what are you looking for? Code structure, quality, extensibility, ...?

Kevin-Mattheus-Moerman commented 3 years ago

Thanks for offering to help @timdewhirst. Yes we ask reviewers to test and evaluate the code. Here is an example of an ongoing review: https://github.com/openjournals/joss-reviews/issues/3611#issuecomment-915658561, as you can see the reviewers get a set of checkboxes at the top to guide them through the process. Here is more information about what we look for during review: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html.

It is good for us to know the background of reviewers and get insight into potential conflicts of interest. If you don't mind, would you be able to share a link to a professional profile, showing your current affiliation e.g. the name of a company or academic institute? Is this your ResearchGate page: https://www.researchgate.net/profile/Tim-Dewhirst ?

timdewhirst commented 3 years ago

I'll take a look at the guidelines, thanks.

researchgate: https://www.researchgate.net/profile/Tim-Dewhirst linkedin: https://www.linkedin.com/in/timdewhirst/

clarka34 commented 3 years ago

@Kevin-Mattheus-Moerman thanks for the invite. I don't feel I can review it properly since I have never coded in C/C++. Good luck finding reviewers since it looks like an interesting project!

Kevin-Mattheus-Moerman commented 3 years ago

@whedon assign @timdewhirst as reviewer

whedon commented 3 years ago

OK, @timdewhirst is now a reviewer

Kevin-Mattheus-Moerman commented 3 years ago

@clarka34, that is understandable, thanks for the reply.

clarka34 commented 3 years ago

@Kevin-Mattheus-Moerman would be happy to check the installation and running of the software though if you need somebody to do that

Kevin-Mattheus-Moerman commented 3 years ago

@clarka34 thanks we'd be happy to have you help here yes. It sounds like installation could be challenging. Would you be okay with being added as an n+1th reviewer? If you manage to install and test the software that would be great, and perhaps @fibreglass2 can guide you through the process if there are any issues? If the installation proves too technical/difficult it would still be great if you could review the short paper, and aspects like the documentation, and perhaps the contributor guidelines etc. You could just leave boxes (which guide reviewers through the process in a review issue) unticked if you cannot evaluate those aspects. Would that be okay?

fibreglass2 commented 3 years ago

perhaps @fibreglass2 can guide you through the process if there are any issues?

Certainly. I just updated the documentation to clarify some points in the installation process for Windows users and provided both single threaded and multi-threaded precompiled programs in the newest release.

clarka34 commented 3 years ago

@Kevin-Mattheus-Moerman I will take a look tomorrow and see how the installation goes. If it goes okay, I would be happy to do the rest!

Kevin-Mattheus-Moerman commented 3 years ago

@clarka34 okay thanks

clarka34 commented 3 years ago

@Kevin-Mattheus-Moerman I worked on the installation today and was able to use both of the precompiled versions (windows + linux) and then compiled it successfully in windows (although with warnings). I did not have any luck compiling in linux, but I think that was a problem with the dependencies. I found a bunch of minor things that could be improved in the documentation. Do I raise issues individually or group them?

Kevin-Mattheus-Moerman commented 3 years ago

@clarka34 thanks for doing that. If you would be okay with it it would be great if I can assign you as a reviewer, it sounds like you'd be able to help.

This issue here is a pre-review issue. Once I have found enough reviewers I will open a dedicated review issue (which will show tickboxes to guide your review). Also we encourage reviewers to open issues on the software repository. You can group things or separate as you see fit. Then please refer to these opened issues in the review issue.

Would it be okay to assign you as a reviewer at this point?

Kevin-Mattheus-Moerman commented 3 years ago

@yosefm @eguvep @dcsale would you be interested in reviewing this work for JOSS?

The review focuses on the software itself, as well as a short paper. The streamlined review process takes place on GitHub (here is an example of an ongoing review: https://github.com/openjournals/joss-reviews/issues/3611#issuecomment-915658561).

Let me know if you are interested. Note we can be flexible regarding time taken to review.

clarka34 commented 3 years ago

@Kevin-Mattheus-Moerman Sure thing! Just to be clear though, I definitely cannot review the code itself

Kevin-Mattheus-Moerman commented 3 years ago

@whedon add @clarka34 as reviewer

whedon commented 3 years ago

OK, @clarka34 is now a reviewer

Kevin-Mattheus-Moerman commented 3 years ago

@Kevin-Mattheus-Moerman Sure thing! Just to be clear though, I definitely cannot review the code itself

@clarka34 thanks, that is fine. We will also include other reviewers that will look into the code structure itself. However, a line-by-line code review is not really expected. If you focus on install, documentation, and evaluation of the functionality, that would be great. I will hopefully recruit one more reviewer before I'll trigger the formal review process. Feel free to start already if you like and post issues on this submission's software repository, please post links to any issues later in the review issue. Thanks again!

Kevin-Mattheus-Moerman commented 3 years ago

@quynhneo @ZIQIANG059 @vitorvilela @lento234 @yosefm @eguvep @dcsale would you be interested in reviewing this work for JOSS?

The review focuses on the software itself, as well as a short paper. The streamlined review process takes place on GitHub (here is an example of an ongoing review: https://github.com/openjournals/joss-reviews/issues/3611#issuecomment-915658561).

Let me know if you are interested. Note we can be flexible regarding time taken to review.

quynhneo commented 3 years ago

@Kevin-Mattheus-Moerman Yeah I would. Which aspect of the code would you like my review?

timdewhirst commented 3 years ago

I've raised a few issues; links are at the bottom of this comment.

As I understand it there were two main questions around the code: does the use of OpenMP help take advantage of multicore processors, and is the code well structured and a suitable foundation for a collaborative project implementing various PIV processing algorithms?

On OpenMP: this appears to work as expected.

On code structure: this is my main area of concern; the code need work to follow best practices in structure, legibility, efficiency and testing. Very little C++ is used, and this is a shame; modern C++ has powerful features for generic and functional style programming which would allow the code to be cleaner, more legible, and just as efficient. Currently, I don't believe it's in good enough shape to act as a foundation for significant collaborative development.

issues:

Kevin-Mattheus-Moerman commented 3 years ago

@whedon add @quynhneo as reviewer