openjournals / joss-reviews

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

[REVIEW]: gLBM: A GPU enabled Lattice Boltzmann Method Library #2555

Closed whedon closed 2 years ago

whedon commented 4 years ago

Submitting author: @rclipp (Rachel Clipp) Repository: https://gitlab.kitware.com/LBM/lattice-boltzmann-solver Version: JOSS Editor: @eloisabentivegna Reviewer: @ianhinder, @gouarin Archive: 10.5281/zenodo.6076998

: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/ef96137ff28121d1d3bb0a36187bd9f9"><img src="https://joss.theoj.org/papers/ef96137ff28121d1d3bb0a36187bd9f9/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/ef96137ff28121d1d3bb0a36187bd9f9/status.svg)](https://joss.theoj.org/papers/ef96137ff28121d1d3bb0a36187bd9f9)

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) by leaving comments 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

@ianhinder & @gouarin, 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.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @eloisabentivegna know.

Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest

Review checklist for @ianhinder

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @gouarin

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 4 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @ianhinder, @gouarin it looks like you're currently assigned to review this paper :tada:.

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

: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

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
Reference check summary:

OK DOIs

- None

MISSING DOIs

- https://doi.org/10.1103/physreve.77.056703 may be missing for title: Straight velocity boundaries in the lattice Boltzmann method
- https://doi.org/10.1103/physreve.81.016311 may be missing for title: Three ways to lattice Boltzmann: a unified time-marching picture
- https://doi.org/10.1109/embc.2018.8513672 may be missing for title: An Interactive, Patient-Specific Virtual Surgical Planning System for Upper Airway Obstruction Treatments
- https://doi.org/10.3389/fninf.2014.00013 may be missing for title: ITK: enabling reproducible research and open science

INVALID DOIs

- None
whedon commented 4 years ago

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

eloisabentivegna commented 3 years ago

@ianhinder @gouarin, could you provide an update on the status of your reviews?

gouarin commented 3 years ago

@eloisabentivegna could you assign me to this review ?

eloisabentivegna commented 3 years ago

@eloisabentivegna could you assign me to this review ?

@gouarin, you are assigned to this review issue. Are you having trouble editing/accessing something?

gouarin commented 3 years ago

@eloisabentivegna It is the first time I make a JOSS review. So, I don't know exactly how it should work. But I can't check the boxes. I try to click on "accept the invite" but the link is expired. I think this is my issue.

Could you send me a new invite?

eloisabentivegna commented 3 years ago

@whedon re-invite @gouarin as reviewer.

Certainly! Let me know if things work now.

whedon commented 3 years ago

OK, the reviewer has been re-invited.

@gouarin please accept the invite by clicking this link: https://github.com/openjournals/joss-reviews/invitations

gouarin commented 3 years ago

Now it works. Thanks! I will make my review next week.

eloisabentivegna commented 3 years ago

Thanks, @gouarin.

ianhinder commented 3 years ago

@ianhinder @gouarin, could you provide an update on the status of your reviews?

@eloisabentivegna, sorry there's not been much progress apart from the initial look over the project. I'll devote some time to it, and as with @gouarin, will aim to complete the review next week.

eloisabentivegna commented 3 years ago

Thanks @ianhinder. I look forward to your reviews.

gouarin commented 3 years ago

Hello @rclipp, @eloisabentivegna,

here is my review of the paper:

ianhinder commented 3 years ago

I've been working on installing and running the code. It was quite difficult; I have a number of issues to report, and will make issues in gitlab and post back here with a list.

ianhinder commented 3 years ago

I have opened 16 issues relating to installation and installation documentation. See https://gitlab.kitware.com/LBM/lattice-boltzmann-solver/-/issues?scope=all&utf8=✓&state=opened&author_username=ianhinder. Not all of these are vital.

I will continue the review next week.

eloisabentivegna commented 3 years ago

@rclipp, have you had a chance to check out the reviewers' comments?

eloisabentivegna commented 3 years ago

@rclipp, have you had a chance to check out the reviewers' comments?

Followed up via email.

eloisabentivegna commented 3 years ago

@gouarin, @ianhinder, have you finished posting your comments on this submission? If so, could you explicitly state your recommendation?

ianhinder commented 3 years ago

Contribution and authorship:

ianhinder commented 3 years ago

Installation:

ianhinder commented 3 years ago

Functionality:

ianhinder commented 3 years ago

Performance:

ianhinder commented 3 years ago

Installation instructions:

ianhinder commented 3 years ago

Example usage:

ianhinder commented 3 years ago

Functionality documentation:

ianhinder commented 3 years ago

Automated tests:

ianhinder commented 3 years ago

Community guidelines:

ianhinder commented 3 years ago

State of the field:

ianhinder commented 3 years ago

Quality of writing:

ianhinder commented 3 years ago

General comments:

ianhinder commented 3 years ago

That's all! I've put each separate issue into a separate comment to make it easier to reply.

Current grade: (3) Requires major revisions

eloisabentivegna commented 3 years ago

@rclipp, did you have a chance to look at the reviewers' recommendations? Could you post an update?

rclipp commented 3 years ago

@eloisabentivegna, We have reviewd the recommendations and we are making changes now to both the paper and repository. We hope to have it finished early next week or sooner. Some of the library comparisons will be difficult due to the 1000 word limit, but we will address as many issues as we can. Thanks!

eloisabentivegna commented 3 years ago

@eloisabentivegna, We have reviewd the recommendations and we are making changes now to both the paper and repository. We hope to have it finished early next week or sooner.

Dear @rclipp, could you post an update?

aaron-bray commented 3 years ago

I have addressed all the issues provided and we should get an update to the paper very soon.

I hope the read me is better structured and has a better level of description for building and running our code

Thanks!

eloisabentivegna commented 3 years ago

@aaron-bray, @rclipp, are you happy for the reviewers to take a look at your new version now?

aaron-bray commented 3 years ago

From a code perspective, yes I believe that it should easier to build and run now.

On Sat, Jan 30, 2021, 2:42 PM Eloisa Bentivegna notifications@github.com wrote:

@aaron-bray https://github.com/aaron-bray, @rclipp https://github.com/rclipp, are you happy for the reviewers to take a look at your new version now?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/2555#issuecomment-770269913, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGTUWHZSFG73ZYQMKRMTHQTS4ROL5ANCNFSM4PW6FQBQ .

eloisabentivegna commented 3 years ago

@ianhinder, @gouarin, are you satisfied with the current version?

ianhinder commented 3 years ago

I'll take another look at it this week. Thanks, @aaron-bray, for addressing the issues!

gouarin commented 3 years ago

Thanks @aaron-bray for this update. I'll try to give my feedback next week.

eloisabentivegna commented 3 years ago

@ianhinder, @gouarin, do you have updates to report?

eloisabentivegna commented 3 years ago

@ianhinder, @gouarin, will you have the chance to complete your reviews shortly? You only seem to have a few unchecked items each, so hopefully this won't require too much longer.

gouarin commented 3 years ago

@eloisabentivegna it seems that the paper has not been modified since my first remarks and it is therefore impossible for me to continue the review procedure as long as my remarks have not been taken into account. When it will be done, I will check the last items.

eloisabentivegna commented 3 years ago

it seems that the paper has not been modified since my first remarks

Thanks, @gouarin. There are some changes to the package itself, if not the paper, that address your points. Could you see if any of those allow you to make progress on the checklist?

@rclipp, @aaron-bray, could you incorporate all suggested changes to your paper? Some are just typo fixes and other easy aspects to correct. It should not take too much work.

eloisabentivegna commented 3 years ago

@whedon generate pdf from branch joss

whedon commented 3 years ago
Attempting PDF compilation from custom branch joss. Reticulating splines etc...
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:

rclipp commented 3 years ago

@gouarin @eloisabentivegna @ianhinder Sorry for the paper updates. I am working on them now and should be able to address them today. Thanks.

rclipp commented 3 years ago

@gouarin @eloisabentivegna @ianhinder We have addressed the paper comments. A quick summary for each comment is below.

There is a typo in the first paragraph: properites -> properties - CORRECTED

Links to the references don't work for me. Is it the same for you? - I am not sure. The references are at the end of the paper and look correct, but if they are supposed to be linked within the paper, then no they are not for us either.

Could you explain more precisely why your software is unique compared to others (Palabos, openLB, ...)? Because, for me, rather than the GPU implementation, they can offer what you offer. - The main advantages are the GPU implementation and the Apache 2.0 license, which enables proprietary development of applications. We have updated this is the paper.

Could you write explicitly what kind of equations you can solve? Is it just Navier-Stokes? - We do not solve the Navier-Stokes, I have added the equations.

What kind of LBM schemes are implemented in your software? And what kind of LBM boundary conditions? This has been added in the paper.

Is it 2D and 3D or just 3D? 3D, and this is added to the paper.

It might be interesting to give an example of a configuration file to better understand how to run a simulation. - We have added significantly more detail about this is the readme and instructions. But we are over the paper limit as is, so I did not add this to the paper.

Do you have studied the performance of your software in MLUPS or other metrics? If so, it could be interesting to have a few words about it. - We have not yet done this.

In the automated verification and validation section, I don't understand if the validation is done by plotting the result and see if it is correct or if it is an automated process. Do you have developed your tools for this part? or do you use available test software? We have written our own software to automatically plot and calculate the error. This has been clarified in the paper.

In the future directions, could you explain more precisely what the next functionalities will be? I have clarified this to some degree. as we are going to focus on improve the accuracy at high mach numbers.

It might be interesting to have an overview of the software structure. - Due to space constraints, I did not go into this.