openjournals / joss-reviews

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

[REVIEW]: SyntheticEddyMethod.jl: A Julia package for the creation of inlet flow conditions for LES #5565

Closed editorialbot closed 1 year ago

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@carlodev<!--end-author-handle-- (Carlo Brunelli) Repository: https://github.com/carlodev/SyntheticEddyMethod.jl Branch with paper.md (empty if default branch): JossSub Version: v0.4.2 Editor: !--editor-->@philipcardiff<!--end-editor-- Reviewers: @atzberg, @akshaysridhar Archive: 10.5281/zenodo.8167205

Status

status

Status badge code:

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

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

@atzberg & @akshaysridhar, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @philipcardiff 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

Checklists

📝 Checklist for @atzberg

📝 Checklist for @akshaysridhar

editorialbot commented 1 year ago

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

For a list of things I can do to help you, just type:

@editorialbot commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@editorialbot generate pdf
editorialbot commented 1 year ago
Software report:

github.com/AlDanial/cloc v 1.88  T=0.02 s (1287.1 files/s, 140428.5 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
TOML                             3            226              1           1053
Julia                           12            224             97            717
Markdown                         7            183              0            493
YAML                             5              3              6            107
TeX                              1              3              0             50
JSON                             1              0              0              1
-------------------------------------------------------------------------------
SUM:                            29            639            104           2421
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 1 year ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1137/141000671 is OK
- 10.1016/j.ijheatfluidflow.2006.02.006 is OK
- 10.1007/s10494-013-9488-2 is OK
- 10.1016/j.ijheatmasstransfer.2019.02.061 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 1 year ago

Wordcount for paper.md is 656

editorialbot commented 1 year ago

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

atzberg commented 1 year ago

Review checklist for @atzberg

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

atzberg commented 1 year ago

Overall, the package is well-organized with good discussions of the methods, implementations, and usage. The package provides implementations of simulation methods from the literature for modeling turbulent flows. Below are a few relatively minor items and typos to fix.

In the SyntheticEddyMethod.jl/README.md:

In the paper.md -> PDF:

In summary, the package is put together well with good discussions of the implemented methods and on the usage of the APIs. After making the minor edits mentioned above, I recommend accepting for JOSS.

carlodev commented 1 year ago

Dear @atzberg thank you very much for your review and all your comments. I have just fixed the typos and rephrased the sentences you pointed out. I appreciate the effort and the positive feedback.

philipcardiff commented 1 year ago

Thank you, @atzberg , for your comprehensive and prompt review.

@carlodev, thank you for addressing these points; we can now wait for @akshaysridhar 's comments.

carlodev commented 1 year ago

I don't see @akshaysridhar as a participant in this issue

philipcardiff commented 1 year ago

I don't see @akshaysridhar as a participant in this issue

Hi @carlodev , you will see @akshaysridhar listed as a reviewer at the top of this issue.

@akshaysridhar has confirmed to me by email that he can review this submission in the coming 2-3 weeks.

akshaysridhar commented 1 year ago

Review checklist for @akshaysridhar

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

akshaysridhar commented 1 year ago

SyntheticEddyMethod.jl is a Julia language implementation of methods published in literature that enable easy generation of turbulent inflow profiles. The package is generally well organised, and satisfies the key requirements listed in the review checklist. The accompanying paper is concise, and the listed features are consistent with the code. A few suggestions for improvement are listed below.

Tests

Documentation

Paper

I believe this submission is suitable for publication following the minor changes listed above!

philipcardiff commented 1 year ago

Thanks @akshaysridhar.

@carlodev, please address the points raised above; feel free to discuss and clarify the points with @akshaysridhar.

carlodev commented 1 year ago

Thank you @akshaysridhar for your insightful comments and suggestions. I have applied all the modifications, feel free to check and verify if I understood your remarks correctly. I also reorganised the test section, and added the commented example.

Concerning the Info: connecting eddies is an indication to the user that Eddies are convected in the VirtualBox. During the testing phase it is printed continuously because the validation/test process is statistics, so a significant number of samples is needed. The statistical validation can take up quite some time, but seeing that statement printed provides feedback about the fact that is running.

akshaysridhar commented 1 year ago

Thanks @carlodev for making the suggested changes. I've checked out the JossSub branch, and have the following suggestions:

(Edited to include checklist / issue tracking. )

carlodev commented 1 year ago

Thanks @akshaysridhar for your suggestions. I have fixed the docs warnings, the typos. I also modified the @info macro as you suggest in order to avoid seeing [ Info: convecting eddies so many times

akshaysridhar commented 1 year ago

Thanks @carlodev for making the suggested changes! @philipcardiff I have completed my review at this stage; I'm happy with the responses to the comments above!

philipcardiff commented 1 year ago

@editorialbot check references

editorialbot commented 1 year ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1137/141000671 is OK
- 10.1016/j.ijheatfluidflow.2006.02.006 is OK
- 10.1007/s10494-013-9488-2 is OK
- 10.1016/j.ijheatmasstransfer.2019.02.061 is OK

MISSING DOIs

- None

INVALID DOIs

- None
philipcardiff commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

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

philipcardiff commented 1 year ago

HI @carlodev , I have proposed a few minor English edits in a pull request: https://github.com/carlodev/SyntheticEddyMethod.jl/pull/20.

Let me know when these have been incorporated.

philipcardiff commented 1 year ago

@carlodev: once you have made the final changes above, please issue a new tagged release of the software (if changed) and archive it (e.g. on Zenodo, figshare, or other). Please then post the version number and archive DOI here.

akshaysridhar commented 1 year ago

@philipcardiff @carlodev Apologies for missing this earlier - but the pdf file doesn't appear to render figure captions correctly - I see that these captions for the two figures are included in the code as :

![Spectra examples at different Turbulent Intensities using tent function as shape function. It refers to the fluctuations in time in one specific point.](images/docs/Spectra.png){ width=50% }
![normalized divergence in a plane using the divergence-free feature. \label{fig:dfsem-plane}](images/docs/Div_free_plane.png){ width=50%  }
carlodev commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

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

carlodev commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

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

carlodev commented 1 year ago

Hi @philipcardiff, I have merged your pull request, fixed the rendering error pointed out by @akshaysridhar . I merged the joss folder with the master, published a new version (0.4.2) and it has been archived on zenodo: 10.5281/zenodo.8167205

philipcardiff commented 1 year ago

Hi @carlodev , I made one more typo fix at https://github.com/carlodev/SyntheticEddyMethod.jl/pull/22/files.

philipcardiff commented 1 year ago

@editorialbot set 10.5281/zenodo.8167205 as archive

editorialbot commented 1 year ago

Done! archive is now 10.5281/zenodo.8167205

carlodev commented 1 year ago

Hi @philipcardiff , thanks for the typo fix, I have merged your pull request

philipcardiff commented 1 year ago

@editorialbot set v0.4.2 as version

editorialbot commented 1 year ago

Done! version is now v0.4.2

philipcardiff commented 1 year ago

@editorialbot recommend-accept

editorialbot commented 1 year ago
Attempting dry run of processing paper acceptance...
editorialbot commented 1 year ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1137/141000671 is OK
- 10.1016/j.ijheatfluidflow.2006.02.006 is OK
- 10.1007/s10494-013-9488-2 is OK
- 10.1016/j.ijheatmasstransfer.2019.02.061 is OK

MISSING DOIs

- None

INVALID DOIs

- None
philipcardiff commented 1 year ago

Hi @kyleniemeyer, this paper is ready for processing.

editorialbot commented 1 year ago

:wave: @openjournals/pe-eics, this paper is ready to be accepted and published.

Check final proof :point_right::page_facing_up: Download article

If the paper PDF and the deposit XML files look good in https://github.com/openjournals/joss-papers/pull/4421, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

philipcardiff commented 1 year ago

Hi @carlodev, please perform one last check of the article proof, and let me know if you find any issues.

carlodev commented 1 year ago

I have checked it, and changed a capital letter in a figure caption

philipcardiff commented 1 year ago

@editorialbot recommend-accept

editorialbot commented 1 year ago
Attempting dry run of processing paper acceptance...
editorialbot commented 1 year ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1137/141000671 is OK
- 10.1016/j.ijheatfluidflow.2006.02.006 is OK
- 10.1007/s10494-013-9488-2 is OK
- 10.1016/j.ijheatmasstransfer.2019.02.061 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 1 year ago

:wave: @openjournals/pe-eics, this paper is ready to be accepted and published.

Check final proof :point_right::page_facing_up: Download article

If the paper PDF and the deposit XML files look good in https://github.com/openjournals/joss-papers/pull/4422, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

kyleniemeyer commented 1 year ago

Looks good to me!

kyleniemeyer commented 1 year ago

@editorialbot accept

editorialbot commented 1 year ago
Doing it live! Attempting automated processing of paper acceptance...