[REVIEW]: directChillFoam: an OpenFOAM application for direct-chill casting

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@blebon@meg-simula<!--end-editor-- Reviewers: @chennachaos, @bzindovic Archive: 10.5281/zenodo.7645448

Status

Status badge code:

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

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

@chennachaos & @bzindovic, 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 @meg-simula 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 @chennachaos

📝 Checklist for @bzindovic

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.08 s (822.0 files/s, 94494.9 lines/s)
--------------------------------------------------------------------------------
Language                      files          blank        comment           code
--------------------------------------------------------------------------------
C++                               8            376            229           1600
C/C++ Header                     13            459            580            889
Python                            8            175            216            542
Markdown                          7             90              0            228
awk                               4             34            120            194
TeX                               1             18              0            186
reStructuredText                  9            203            254            181
YAML                              4             22             26            149
sed                               2             34             47             98
Bourne Again Shell                4             23             50             49
DOS Batch                         1              8              1             26
make                              1              4              7              9
--------------------------------------------------------------------------------
SUM:                             62           1446           1530           4151
--------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository

editorialbot commented 1 year ago

Wordcount for paper.md is 635

editorialbot commented 1 year ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/S0364-5916(02)00037-8 is OK
- 10.1201/9781420062823 is OK
- 10.1201/b17270 is OK
- 10.1016/j.ijheatmasstransfer.2017.10.033 is OK
- 10.1007/s11663-017-1016-7 is OK
- 10.3390/ma12193262 is OK
- 10.1016/j.ultsonch.2019.02.002 is OK
- 10.1016/j.apm.2019.08.032 is OK
- 10.1007/s11663-017-1016-7 is OK
- 10.1080/10426914.2014.921698 is OK
- 10.1016/j.jmatprotec.2021.117170 is OK
- 10.1016/j.jmrt.2021.03.061 is OK
- 10.1016/j.apm.2017.09.034 is OK
- 10.4028/www.scientific.net/MSF.765.291 is OK
- 10.1016/j.jmatprotec.2021.117116 is OK

MISSING DOIs

- None

INVALID DOIs

- None

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:

chennachaos commented 1 year ago

Review checklist for @chennachaos

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the https://github.com/blebon/directChillFoam?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@blebon) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[ ] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
[x] Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
[ ] Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
[x] Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
[x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

arfon commented 1 year ago

:wave: @chennachaos & @bzindovic – just checking in here to see how you are both getting along with your reviews? @chennachaos – I see you have started yours but @bzindovic it looks like you are yet to start. Is there anything I can do to help move things along here?

bzindovic commented 1 year ago

:wave: @chennachaos & @bzindovic – just checking in here to see how you are both getting along with your reviews? @chennachaos – I see you have started yours but @bzindovic it looks like you are yet to start. Is there anything I can do to help move things along here?

I apologize for not completing my review earlier due to personal reasons. I'll complete the review by Friday evening.

bzindovic commented 1 year ago

Review checklist for @bzindovic

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the https://github.com/blebon/directChillFoam?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@blebon) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
[x] Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
[x] Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
[x] Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
[x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

bzindovic commented 1 year ago

I congratulate the author on an interesting package, I think it will be a great contribution to the overall OpenFOAM ecosystem of solvers. After reading the manuscript and going through the source code, examples and docs, my observations are as follows:

State of the field is actually embedded inside the statement of need. Maybe it should be in a separate section?
References section includes a reference to the OpenFOAM-dev repository but states the authors as Weller & Jasak (2022). As the repository owner is OpenFOAM Foundation, maybe the reference should cite it as an "author/owner"? Or maybe it was meant to include this reference: Weller, H.G., Tabor, G., Jasak, H., Fureby, C. (1998) A tensorial approach to computational continuum mechanics using object-oriented techniques, Comput. Phys., Volume 12, Issue 6, Pages 620-631?

Once again, I compliment the author for crafting a valuable tool on top of OpenFOAM.

blebon commented 1 year ago

@bzindovic Happy New Year 2023 and thank you for your review and helpful comments.

Thanks for the suggestion. I will separate the state of the field from the statement of need.
I will update the reference to OpenFOAM with the suggested paper.

@arfon Happy New Year 2023. ~Should I implement these suggestions immediately, or should I wait for @chennachaos 's comments before making any change?~ Edit: Committed the suggested changes.

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

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

blebon commented 1 year ago

Fixed references: added pages where they were missing and made my name consistent across my referenced publications. Added links to grants in acknowledgement section.

meg-simula commented 1 year ago

Hi @bzindovic and @blebon, and happy new year! Thanks for the review and prompt responses.

meg-simula commented 1 year ago

@chennachaos Have you had a chance to look at this further?

bzindovic commented 1 year ago

Dear all, I wish you Happy and prosperous New Year. @blebon with the last modification of the manuscript, you've addressed all my remarks so I'm marking the last point of the review as complete.

meg-simula commented 1 year ago

@bzindovic Thank you very much for the constructive review and updates.

meg-simula commented 1 year ago

@chennachaos We are now waiting for your review, could you please give us an update?

chennachaos commented 1 year ago

Hi @meg-simula, I am extremely sorry for the delay. I will complete the review by the end of next week.

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

chennachaos commented 1 year ago

Hi @blebon, I think this library is a great add-on to OpenFOAM. The functionality is described well. However, at the moment, it lacks in content in terms of documentation.

One tutorial is not sufficient. Additional tutorials, preferably one 3D test case, need to be added to demonstrate the features further.
Update the existing tutorial with grid plots with appropriate annotations. Also, add the details of runtime at the execution stage.
It is not quite clear what the Python helper script does. Add some details on this. Also, it is not clear how to set up different problems. Additional details on this would be useful for the users interested in using this library.
The library lacks automated tests to verity/test its functionality.
Community guidelines on contributing to the library and reporting issues are missing.

blebon commented 1 year ago

@chennachaos, thank you for the constructive comments. I am working on your comments and hope to resolve all of them in a timely manner.

While going through them, I had a few questions regarding some of your comments:

"It is not quite clear what the Python helper script does.": I already have documentation for the Python scripts that are used in the tutorials: https://blebon.com/directChillFoam/tutorials/modules.html. I am not quite sure what to add more here. Would you like to see a long description for each function?
"The library lacks automated tests to verity/test its functionality.": Would a benchmark validation test like this be adequate to address your comment?
The community guidelines were in the .github folder. Would you like to see more description in there or do you think they should have been placed somewhere else. For now, I have added links to these in the root README.md.

Thanks again for the comments. I am working on adding more tutorials, including a 3D case. I will add figures for describing the meshes that are used and a rough estimate of the run time on my test virtual environment (this will go in the Sphinx documentation). I will also expand the documentation to help a new OpenFOAM user to setup a new case. The latter might take some time but hope to get everything sorted by early next week.

chennachaos commented 1 year ago

@blebon,

I think this is good enough. You may consider pointing to this page from the place where the Python script is used.
This test looks like an integration test. It's better to have some unit tests to test the key functions.
The main README.md file or the documentation would be a better place for the community guidelines.

I hope that clarifies.

blebon commented 1 year ago

@chennachaos, thanks again for your feedback. I have made the following changes and I hope that they adequately address your comments:

[ ] One tutorial is not sufficient. Additional tutorials, preferably one 3D test case, need to be added to demonstrate the features further. I have added two more tutorials cases:
1. https://github.com/blebon/directChillFoam/tree/master/tutorials/heatTransfer/directChillFoam/Lebon2020 which is a 3D case.
2. https://github.com/blebon/directChillFoam/tree/master/tutorials/heatTransfer/directChillFoam/Subroto2021 which is another axisymmetric case with a ternary alloy.
[ ] Update the existing tutorial with grid plots with appropriate annotations.
I have added a schematic with annotations https://blebon.com/directChillFoam/tutorials/Vreeman2002.html#vreeman2002-bc. Similar annotated figures have been added to the other two tutorials.
[ ] Also, add the details of runtime at the execution stage. I have included the following statement (and corresponding statements for the other two tutorials) before the pre-processing section:

The execution time of this tutorial case is around 15 minutes when run in serial on a node with a 2.10 GHz processor.
[ ] It is not quite clear what the Python helper script does. Add some details on this.
I have added code documentation to cylinder.py, e.g. https://blebon.com/directChillFoam/tutorials/modules.html#module-Vreeman2002.system.cylinder. A link to the python function documentation section has been added to the mesh generation section of the tutorial (idem for other two tutorials).
[ ] Also, it is not clear how to set up different problems. Additional details on this would be useful for the users interested in using this library. I have expanded the tutorial description in the tutorials section: https://blebon.com/directChillFoam/tutorials/index.html.
[ ] The library lacks automated tests to verity/test its functionality. I have added unit tests in https://github.com/blebon/directChillFoam/tree/master/tests. These tests are included in the github workflow yaml file:

- name: Run unit tests
      shell: bash
      working-directory: tests
      continue-on-error: true
      run: |
        source /opt/openfoam9/etc/bashrc || true
        mkdir -p $FOAM_RUN
        ln -s "$(realpath ../applications)" $WM_PROJECT_USER_DIR
        ln -s "$(realpath ../src)" $WM_PROJECT_USER_DIR
        pytest
        ./Alltest

[ ] Community guidelines on contributing to the library and reporting issues are missing. I have added links to the community and issue guidelines in README.md. The templates and checkboxes also appear in the issue textbox when creating a new bug report/feature request here: https://github.com/blebon/directChillFoam/issues/new/choose.

Edit: Formatting.

chennachaos commented 1 year ago

Hi @blebon, thanks for the updates! You have addressed all the concerns.

chennachaos commented 1 year ago

Hi @meg-simula, I have completed my review. I am happy to recommend this submission for publication in JOSS.

meg-simula commented 1 year ago

Thanks for your review @chennachaos!

meg-simula commented 1 year ago

@editorialbot generate pdf

meg-simula commented 1 year ago

@editorialbot check references

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:

editorialbot commented 1 year ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/S0364-5916(02)00037-8 is OK
- 10.1201/9781420062823 is OK
- 10.1201/b17270 is OK
- 10.1016/j.ijheatmasstransfer.2017.10.033 is OK
- 10.1016/j.apm.2017.09.034 is OK
- 10.1007/s11661-017-4238-z is OK
- 10.3390/ma12193262 is OK
- 10.1016/j.ultsonch.2019.02.002 is OK
- 10.1016/j.apm.2019.08.032 is OK
- 10.1007/s11663-017-1016-7 is OK
- 10.1007/BF02651234 is OK
- 10.1080/10426914.2014.921698 is OK
- 10.1016/j.jmatprotec.2021.117170 is OK
- 10.1016/j.jmrt.2021.03.061 is OK
- 10.1007/s11661-018-4632-1 is OK
- 10.1115/1.1482089 is OK
- 10.1063/1.168744 is OK
- 10.4028/www.scientific.net/MSF.765.291 is OK
- 10.1016/j.jmatprotec.2021.117116 is OK

MISSING DOIs

- None

INVALID DOIs

- https://doi.org/10.1016/0017-9310(87)90094-9 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1016/S0017-9310(99)00174-X is INVALID because of 'https://doi.org/' prefix

meg-simula commented 1 year ago

@blebon Thanks for addressing all the reviewers' comments. I just have one minor additional point: should it be "exits" rather than "exists" in the introduction?

meg-simula commented 1 year ago

Next, could you please:

[ ] Make a tagged release of your software, and list the version tag of the archived version here.
[ ] Archive the reviewed software in Zenodo or a similar service (e.g., figshare, an institutional repository)
[ ] Check the archival deposit (e.g., in Zenodo) has the correct metadata. This includes the title (should match the paper title) and author list (make sure the list is correct and people who only made a small fix are not on it). You may also add the authors' ORCID.
[ ] Please list the DOI of the archived version here.

I can then move forward with recommending acceptance of the submission.

blebon commented 1 year ago

@chennachaos and @bzindovic Thank you for your review. Your comments improved the repository and submission.

blebon commented 1 year ago

@editorialbot generate pdf

blebon 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.1016/S0364-5916(02)00037-8 is OK
- 10.1016/0017-9310(87)90094-9 is OK
- 10.1201/9781420062823 is OK
- 10.1201/b17270 is OK
- 10.1016/j.ijheatmasstransfer.2017.10.033 is OK
- 10.1016/j.apm.2017.09.034 is OK
- 10.1007/s11661-017-4238-z is OK
- 10.3390/ma12193262 is OK
- 10.1016/j.ultsonch.2019.02.002 is OK
- 10.1016/j.apm.2019.08.032 is OK
- 10.1007/s11663-017-1016-7 is OK
- 10.1007/BF02651234 is OK
- 10.1080/10426914.2014.921698 is OK
- 10.1016/j.jmatprotec.2021.117170 is OK
- 10.1016/j.jmrt.2021.03.061 is OK
- 10.1007/s11661-018-4632-1 is OK
- 10.1016/S0017-9310(99)00174-X is OK
- 10.1115/1.1482089 is OK
- 10.1063/1.168744 is OK
- 10.4028/www.scientific.net/MSF.765.291 is OK
- 10.1016/j.jmatprotec.2021.117116 is OK

MISSING DOIs

- None

INVALID DOIs

- None

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:

blebon commented 1 year ago

@meg-simula Thank you for editing this submission. I have fixed the typo in the introduction and fixed the references that the editorial bot complained about above.

Regarding your final comments:

The new tag is OF9.0.3-JOSS: https://github.com/blebon/directChillFoam/releases/tag/OF9.0.3-JOSS
This tag is archived in Zenodo at https://zenodo.org/record/7645448
I have updated the metadata in Zenodo. I am the sole author of this software, so the authors list is correct.
The doi of the archive is 10.5281/zenodo.7645448

meg-simula commented 1 year ago

@editorialbot set 10.5281/zenodo.7645448 as archive

editorialbot commented 1 year ago

Done! Archive is now 10.5281/zenodo.7645448

meg-simula 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.1016/S0364-5916(02)00037-8 is OK
- 10.1016/0017-9310(87)90094-9 is OK
- 10.1201/9781420062823 is OK
- 10.1201/b17270 is OK
- 10.1016/j.ijheatmasstransfer.2017.10.033 is OK
- 10.1016/j.apm.2017.09.034 is OK
- 10.1007/s11661-017-4238-z is OK
- 10.3390/ma12193262 is OK
- 10.1016/j.ultsonch.2019.02.002 is OK
- 10.1016/j.apm.2019.08.032 is OK
- 10.1007/s11663-017-1016-7 is OK
- 10.1007/BF02651234 is OK
- 10.1080/10426914.2014.921698 is OK
- 10.1016/j.jmatprotec.2021.117170 is OK
- 10.1016/j.jmrt.2021.03.061 is OK
- 10.1007/s11661-018-4632-1 is OK
- 10.1016/S0017-9310(99)00174-X is OK
- 10.1115/1.1482089 is OK
- 10.1063/1.168744 is OK
- 10.4028/www.scientific.net/MSF.765.291 is OK
- 10.1016/j.jmatprotec.2021.117116 is OK

MISSING DOIs

- None

INVALID DOIs

- None

meg-simula commented 1 year ago

@blebon @chennachaos @bzindovic Thank you all for your efforts on this review! I have now recommended this paper for acceptance to the JOSS editors-in-chief, and we will await their evaluation.

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/3974, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

openjournals / joss-reviews