uw-comphys / openccm

OpenCCM is a CFD-based compartment modelling software package. It is primarily intended for convection dominated reactive flows which feature a weak or one-way coupling between the reactive species and the carrier fluid, i.e. the reaction does not substantially influence the fluid flow over the course of the simulation.
https://uw-comphys.github.io/openccm/
GNU Lesser General Public License v2.1
0 stars 4 forks source link

JOSS Review - comments on paper #43

Closed speth closed 1 month ago

speth commented 3 months ago

I appreciate the opportunity to review this tool for JOSS (https://github.com/openjournals/joss-reviews/issues/6963). OpenCCM seems like it could be a useful tool in a number of different areas. Besides the other issues I opened regarding the software, I wanted to offer some comments / suggestions for the paper itself:

Alex-Vasile commented 3 months ago

In the description of the user interface, the input file is described as "text based," but is there any more structure to it than that? An example would aid in understanding.

The main directory contains two example config files, CONFIG and CONFIG_REACTIONS. Would indicating their existence and pointing the reader to it address this comment?

speth commented 2 months ago

The main directory contains two example config files, CONFIG and CONFIG_REACTIONS. Would indicating their existence and pointing the reader to it address this comment?

Does the CONFIG file in the repository root describe all of the available configuration options? If so, then I would say yes, pointing to these files is sufficient. Repeating this information in the documentation website would also help clarify how to use the software.

speth commented 2 months ago

Something else I noticed while looking at the reactions config file for the pipe_with_recirc_2d example is that it uses species names like Na2CO3, while the the "Reaction Configuration File" section of the paper suggests that only letters are allowed in species names, requiring these species names to be replaced with abstract ones like a or b. Is this a recent change to the code to support a more natural naming convention?

Alex-Vasile commented 2 months ago

There was a recent code change to improve that, the manuscript was update to reflect it, I forgot to go back to the original submission page an regenerate the manuscript. That's now been done.

speth commented 2 months ago

Thank you, I think all of my original comments have been addressed. I have just two more small items based on looking at the revised manuscript:

Alex-Vasile commented 2 months ago

Thank you, I think all of my original comments have been addressed. I have just two more small items based on looking at the revised manuscript:

  • The sample section of the input file (lines 74-84) runs off the right side of the page, so I suppose you need to manually wrap the comments for the sake of the manuscript.
  • I'm not sure what is meant by lines 149-150, "Input/Output Concentrations for ‘NaCl’." etc.

I've added done the word wrap manually. The "Input/Output concentrations" is Alt text for the figures that should have been there. That was an oversight on my part; when I update the specie names from a, b, etc. to NaCl and the like I forgot to change the file names referenced in the manuscript to reflected those changes.

Alex-Vasile commented 1 month ago

@speth Has everything in this issue been addressed?

speth commented 1 month ago

Yes, I believe so.