Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JOSS Reviewer 2 (v2) #1120

Closed
8 tasks done
rafmudaf opened this issue Jun 22, 2023 · 4 comments
Closed
8 tasks done

JOSS Reviewer 2 (v2) #1120

rafmudaf opened this issue Jun 22, 2023 · 4 comments

Comments

@rafmudaf
Copy link
Contributor

rafmudaf commented Jun 22, 2023

Hi ERF Team,

I've started the second review for the JOSS submission (openjournals/joss-reviews#5202), and I'll add review comments here on an ongoing basis.

Software Paper

Overall, this is very good and reads nicely. It's both comprehensive and concise which is difficult to achieve.

  • L145: Spell out adaptive mesh refinement since AMR here is referring to the concept but it hasn't been introduced as "AMR" yet
  • L128 Cite WPS, if possible, or WRF, if relevant. I didn't see a preferred citation format on either the WPS or WRF cite, but at least it would be good to cite the repo.

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?
My takeaway from the paper is that the primary need is for a new software that leverages modern and future hardware. However, I can deduce that there's some need to have an atmospheric model more closely related to wind energy analysis since it's mentioned in the summary and the statement of need. However, this point is not clear, and I think it's worth clarifying ERF's relevance to wind energy.

  • Summary: Maybe include ERF's relevance to wind energy directly in the first line. Otherwise, that ERF is relevant to wind energy kind of comes out of left field in the third sentence.
  • Statement of need: Similarly, it would be good to strengthen the point about ERF's relevance to wind energy in the statement of need. What are the "standard discretizations and basic features ... relevant to wind energy"?
  • Wind energy section: Possibly both of the above points would be resolved by including a section on features specific to wind energy under ERF Features

State of the field: Do the authors describe how this software compares to other commonly-used packages?

  • Similar to the section above, the comparison to other models is well-developed for parallelization, but it's not clear how ERF differentiates from other models with respect to its use in wind energy analysis.

Documentation

A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
This page is amazing: https://erf.readthedocs.io/en/latest/Applications_Requirements.html. Really well done on that, and maybe consider elevating this or a summary version to somewhere more prominent like the main page and readme.

Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).

  • I see User Guide / Inputs and User Guide/ Getting Started / Running which are both very helpful. A bit of information that's missing, though, is a description of the concept that each problem-type has its own executable. I don't think it's necessary to describe each executable in Exec, but it would help me in understanding ERF to understand the idea behind Exec as opposed to having a single executable to run any problem.
  • As an aside and not required for review, it would be nice to have a description of each executable in Exec. A README in each directory would go far, or a listing in the documentation site describing each and maybe demonstrating their results.

Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?

  • The link to Doxygen API docs is somewhat hidden on the main page. It would be helpful to include a link in the menu on the left possibly under "Code of Conduct". Just a suggestion, not a requirement for the review.
@rafmudaf
Copy link
Contributor Author

@asalmgren I've added some additional comments to the original post above, just heads up!

@asalmgren
Copy link
Collaborator

Thanks @rafmudaf . I believe we have now addressed all of the items above, including the suggestions as well as requirements :-) For the example usage we have added a short section in the Building subsection about the paradigm for how the Exec directory is laid out and how to build with gmake vs cmake. In addition, we have restructured Exec to make it more clear which are "science" tests vs "regression tests" vs tests of features under development.

@rafmudaf
Copy link
Contributor Author

Great work @asalmgren. Thanks for addressing my comments, I'm happy with the state of the JOSS submission and will update the review accordingly. Feel free to close this issue.

@asalmgren
Copy link
Collaborator

Thanks for all the great feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants