search

openjournals / joss-reviews

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

[REVIEW]: ORION2: A magnetohydrodynamics code for star formation #3771

Closed whedon closed 2 years ago

whedon commented 2 years ago

Submitting author: @soffner (Stella Offner) Repository: https://bitbucket.org/orionmhdteam/orion2_release1/src/master/ Version: 1.0 Editor: @dfm Reviewer: @zingale, @changgoo Archive: 10.5281/zenodo.5791188

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

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

@zingale & @changgoo, 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 @dfm 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 @zingale

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @changgoo

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 2 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @zingale, @changgoo 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 2 years ago

Wordcount for paper.md is 836

whedon commented 2 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1093/mnras/stz653 is OK
- 10.1093/mnras/stx2611 is OK
- 10.1093/mnras/stv1437 is OK
- 10.1088/0004-637X/747/1/22 is OK
- 10.1016/j.jcp.2007.07.035 is OK
- 10.1016/j.jcp.2007.09.032 is OK
- 10.1086/421935 is OK
- 10.1088/0004-637X/745/2/139 is OK
- 10.1086/305329 is OK
- 10.1016/s0377-0427(99)00156-9 is OK
- 10.1086/310975 is OK
- 10.1088/0004-637X/740/2/107 is OK
- 10.1088/0004-637X/811/2/146 is OK
- 10.1086/426051 is OK
- 10.1086/590238 is OK
- 10.1088/0004-6256/136/1/404 is OK
- 10.1088/0004-637X/703/1/131 is OK
- 10.1088/0004-637X/693/1/914 is OK
- 10.1088/0004-637X/704/2/L124 is OK
- 10.1093/mnras/stu052 is OK
- 10.1038/nature13662 is OK
- 10.1088/0004-637X/783/1/50 is OK
- 10.1038/s41550-018-0566-1 is OK
- 10.3847/1538-4357/ab584b is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 2 years ago

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

whedon commented 2 years ago 
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=7.24 s (200.5 files/s, 61814.4 lines/s)
--------------------------------------------------------------------------------
Language                      files          blank        comment           code
--------------------------------------------------------------------------------
C++                             516          32029          31720         184833
C/C++ Header                    504          17559          35231          64419
C                               127           6463          10161          21534
Python                           46           1326           1426           5013
Fortran 77                       15            325           3158           4388
IDL                              50           1707           2938           4257
Perl                             25            900           1755           3580
make                             79            857            931           1599
SWIG                              7            827              0           1472
reStructuredText                 33            697            171           1057
HTML                              1            390              1            881
INI                              14            248              0            824
Bourne Shell                     17             99            114            560
Bourne Again Shell                3             57            170            439
TeX                               1             45              0            428
diff                              1             34            160            232
MATLAB                            1             24              5            134
Markdown                          1             16              0            117
C Shell                           8             28              3            103
awk                               1              1              4             37
Fortran 90                        1              6              0             28
CMake                             1              5             11             10
--------------------------------------------------------------------------------
SUM:                           1452          63643          87959         295945
--------------------------------------------------------------------------------

Statistical information for the repository '80c01aa4e10b489d5f3ff26a' was
gathered on 2021/09/27.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Offner                           1        298269              0          100.00

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
Stella Offner            298269          100.0          0.0               15.55
dfm commented 2 years ago

@soffner, @zingale, @changgoo – This is the review thread for the paper. All of our communications will happen here from now on. Thanks again for agreeing to participate!

Please read the "Reviewer instructions & questions" in the first comment above.

Both reviewers have checklists at the top of this thread (in that first comment) with the JOSS requirements. As you go over the submission, please check any items that you feel have been satisfied. There are also links to the JOSS reviewer guidelines.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention openjournals/joss-reviews#3771 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for the review process to be completed within about 4-6 weeks but please make a start well ahead of this as JOSS reviews are by their nature iterative and any early feedback you may be able to provide to the author will be very helpful in meeting this schedule.

changgoo commented 2 years ago

@dfm @soffner How can I create an issue on the ORION2 repository?

Currently, the code is hosted at https://bitbucket.org/orionmhdteam/orion2_release1/src/master/ but the documentation indicates a different url at https://bitbucket.org/orionmhd/orion2/ which is not there yet. Will the repo eventually move to the latter point? I don't know whether the review should be conducted on the final repo or not.

dfm commented 2 years ago

@changgoo: Good points! Ignore my comment about mentioning this issue since ORION2 is on bitbucket - I just posted my usual canned comment, sorry!

Once @soffner clarifies your question about which repository to use, feel free to open issues over on bitbucket if you have an account. If not, you're also welcome to just use this review issue to share your comments. If it's not clear how to open issues on the bitbucket repo, that's something that we'd hope to catch as part of this review.

soffner commented 2 years ago

Thank you @changgoo https://github.com/changgoo: the correct repository is https://bitbucket.org/orionmhdteam/orion2_release1/src/master/

I'll update it in the documentation.

On Mon, Sep 27, 2021 at 6:35 PM Dan Foreman-Mackey @.***> wrote:

@changgoo https://github.com/changgoo: Good points! Ignore my comment about mentioning this issue since ORION2 is on bitbucket - I just posted my usual canned comment, sorry!

Once @soffner https://github.com/soffner clarifies your question about which repository to use, feel free to open issues over on bitbucket if you have an account. If not, you're also welcome to just use this review issue to share your comments. If it's not clear how to open issues on the bitbucket repo, that's something that we'd hope to catch as part of this review.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/3771#issuecomment-928446506, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFACU5HTVCOTAPBR5OEEGTUED5U5ANCNFSM5E3I6I3A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

changgoo commented 2 years ago

Still, I only see the Jira Issues tab, which seems to be inactive. In the meantime, I will put some comments here instead.

changgoo commented 2 years ago

Based on the checklist here, it might be recommended to rename COPYING to LICENSE.

changgoo commented 2 years ago

I failed to compile the code. When I run this command

python $ORION2_DIR/setup.py --with-orion2 --no-gui

I first get an error message complaining about undefined variable WITH_CHOMBO. Copying the following lines within the no_gui function solves this problem. (or this can be moved outside functions so that defined globally.) https://bitbucket.org/orionmhdteam/orion2_release1/src/dcd893ca943a350199ded758fd085c95c6c6035e/Tools/Python/ut.py#lines-51

Now, I get an error message

FileNotFoundError: [Errno 2] No such file or directory: '/Users/ckim/Sources/orion2_release1//Src/Templates/orion2.ini'

Apparently, there is only pluto.ini in the folder.

soffner commented 2 years ago

Thanks, Chang-Goo.

LICENSE renaming fixed.

I'm looking into why the issue tracker is not accessible.

It looks like you are trying to compile in the Template directory. Try compiling one of the test problems and compile in that directory. No files should need to be added there.

On Thu, Sep 30, 2021 at 3:39 PM Chang-Goo Kim @.***> wrote:

I failed compile the code. When I run this command

python $ORION2_DIR/setup.py --with-orion2 --no-gui

I first get an error message complaining about undefined variable WITH_CHOMBO. Copying the following lines within the no_gui function solves this problem. (or this can be moved outside functions so that defined globally.)

https://bitbucket.org/orionmhdteam/orion2_release1/src/dcd893ca943a350199ded758fd085c95c6c6035e/Tools/Python/ut.py#lines-51

Now, I get an error message

FileNotFoundError: [Errno 2] No such file or directory: '/Users/ckim/Sources/orion2_release1//Src/Templates/orion2.ini'

Apparently, there is only pluto.ini in the folder.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/3771#issuecomment-931647617, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFACU4DIB6GV27HUY5EJO3UETDGRANCNFSM5E3I6I3A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

soffner commented 2 years ago

The issue tracker is enabled for public use now. Let me know if you still can't see it.

On Thu, Sep 30, 2021 at 4:41 PM Stella Offner @.***> wrote:

Thanks, Chang-Goo.

LICENSE renaming fixed.

I'm looking into why the issue tracker is not accessible.

It looks like you are trying to compile in the Template directory. Try compiling one of the test problems and compile in that directory. No files should need to be added there.

On Thu, Sep 30, 2021 at 3:39 PM Chang-Goo Kim @.***> wrote:

I failed compile the code. When I run this command

python $ORION2_DIR/setup.py --with-orion2 --no-gui

I first get an error message complaining about undefined variable WITH_CHOMBO. Copying the following lines within the no_gui function solves this problem. (or this can be moved outside functions so that defined globally.)

https://bitbucket.org/orionmhdteam/orion2_release1/src/dcd893ca943a350199ded758fd085c95c6c6035e/Tools/Python/ut.py#lines-51

Now, I get an error message

FileNotFoundError: [Errno 2] No such file or directory: '/Users/ckim/Sources/orion2_release1//Src/Templates/orion2.ini'

Apparently, there is only pluto.ini in the folder.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/3771#issuecomment-931647617, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFACU4DIB6GV27HUY5EJO3UETDGRANCNFSM5E3I6I3A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

changgoo commented 2 years ago

Okay, now I see the issue tracker. Let me create an issue there.

whedon commented 2 years ago

:wave: @changgoo, please update us on how your review is going (this is an automated reminder).

whedon commented 2 years ago

:wave: @zingale, please update us on how your review is going (this is an automated reminder).

zingale commented 2 years ago

it's on my radar, but as I noted when I accepted, I'll need a few weeks

dfm commented 2 years ago

@zingale: Yeah, I remember. The bot just sends this message on a timer no matter what. Thanks again!

changgoo commented 2 years ago

Sorry for the delayed progress. I've been distracted by other things, but now I'm tying to finish the review. Before deep dive into the software itself, I have a major suggestion about the JOSS paper (and also repository README) in the description of the development history. As the code documentation describes, the ORION2 code is largely based on the PLUTO code base. It adopts a large portion of the PLUTO code, while the implementation of a new AMR MHD module using the constraint transport method and subsequent developments distinguishes the ORION2 code from the PLUTO code. Since the code repository doesn't have such information, it would be necessary to clarify the development history and the contribution of the ORION2 code in the JOSS paper.

Although the PLUTO code is a (kind of) public code, it requires a registration step to get the source code. It is unclear to me whether it is okay to make ORION2 fully open-source given the policy adopted in PLUTO. If the ORION2 team already obtained the consent, it may also be good to be clarified.

soffner commented 2 years ago

@whedon generate pdf

On Mon, Sep 27, 2021 at 2:22 PM whedon @.***> wrote:

Submitting author: @soffner https://github.com/soffner (Stella Offner http://orcid.org/0000-0003-1252-9916) Repository: https://bitbucket.org/orionmhdteam/orion2_release1/src/master/ https://bitbucket.org/orionmhdteam/orion2_release1/src/master/ Version: v1.0.0 Editor: @dfm https://github.com/dfm Reviewer: @zingale https://github.com/zingale, @changgoo https://github.com/changgoo Archive: Pending

⚠️ JOSS reduced service mode ⚠️

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 https://blog.joss.theoj.org/2020/05/reopening-joss. Status

[image: status] https://joss.theoj.org/papers/c8c824a316e16f6929ad490ee93d9f5d

Status badge code:

HTML:

Markdown: status

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository https://bitbucket.org/orionmhdteam/orion2_release1/src/master/ 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

@zingale https://github.com/zingale & @changgoo https://github.com/changgoo, 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 @dfm https://github.com/dfm 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 @zingale https://github.com/zingale

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission https://bitbucket.org/orionmhdteam/orion2_release1/src/master/. ✨ Conflict of interest

Code of Conduct

General checks

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • 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

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
  • 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

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • 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 and who the target audience is?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • 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 https://rmarkdown.rstudio.com/authoring_bibliographies_and_citations.html#citation_syntax ?

Review checklist for @changgoo https://github.com/changgoo

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission https://bitbucket.org/orionmhdteam/orion2_release1/src/master/. ✨ Conflict of interest

Code of Conduct

General checks

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • 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

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
  • 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

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • 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 and who the target audience is?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • 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 https://rmarkdown.rstudio.com/authoring_bibliographies_and_citations.html#citation_syntax ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/3771, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFACU3UFUMNO7RDVBWTM23UEC757ANCNFSM5E3I6I3A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

whedon commented 2 years ago

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

soffner commented 2 years ago

@whedon generate pdf

On Tue, Nov 9, 2021 at 1:49 PM Stella Offner @.***> wrote:

@whedon generate pdf

On Mon, Sep 27, 2021 at 2:22 PM whedon @.***> wrote:

Submitting author: @soffner https://github.com/soffner (Stella Offner http://orcid.org/0000-0003-1252-9916) Repository: https://bitbucket.org/orionmhdteam/orion2_release1/src/master/ https://bitbucket.org/orionmhdteam/orion2_release1/src/master/ Version: v1.0.0 Editor: @dfm https://github.com/dfm Reviewer: @zingale https://github.com/zingale, @changgoo https://github.com/changgoo Archive: Pending

⚠️ JOSS reduced service mode ⚠️

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 https://blog.joss.theoj.org/2020/05/reopening-joss. Status

[image: status] https://joss.theoj.org/papers/c8c824a316e16f6929ad490ee93d9f5d

Status badge code:

HTML:

Markdown: status

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository https://bitbucket.org/orionmhdteam/orion2_release1/src/master/ 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

@zingale https://github.com/zingale & @changgoo https://github.com/changgoo, 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 @dfm https://github.com/dfm 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 @zingale https://github.com/zingale

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission https://bitbucket.org/orionmhdteam/orion2_release1/src/master/. ✨ Conflict of interest

Code of Conduct

General checks

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • 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

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
  • 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

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • 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 and who the target audience is?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • 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 https://rmarkdown.rstudio.com/authoring_bibliographies_and_citations.html#citation_syntax ?

Review checklist for @changgoo https://github.com/changgoo

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission https://bitbucket.org/orionmhdteam/orion2_release1/src/master/. ✨ Conflict of interest

Code of Conduct

General checks

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • 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

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
  • 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

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • 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 and who the target audience is?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • 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 https://rmarkdown.rstudio.com/authoring_bibliographies_and_citations.html#citation_syntax ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/3771, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFACU3UFUMNO7RDVBWTM23UEC757ANCNFSM5E3I6I3A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

whedon commented 2 years ago

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

soffner commented 2 years ago

@changgoo: Thanks for bringing up this point. We have added PLUTO citations to the documentation and JOSS paper as well as made the connection between PLUTO and ORION2 more explicit. Note that PLUTO is released under the GNU public license. The download link you sent appears to be so that the PLUTO developers can collect information on users. It is not related to licensing.

On Mon, Nov 8, 2021 at 12:48 PM Chang-Goo Kim @.***> wrote:

Sorry for the delayed progress. I've been distracted by other things, but now I'm tying to finish the review. Before deep dive into the software itself, I have a major suggestion about the JOSS paper (and also repository README) in the description of the development history. As the code documentation describes, the ORION2 code is largely based on the PLUTO code base. It adopts a large portion of the PLUTO code, while the implementation of a new AMR MHD module using the constraint transport method and subsequent developments distinguishes the ORION2 code from the PLUTO code. Since the code repository doesn't have such information, it would be necessary to clarify the development history and the contribution of the ORION2 code in the JOSS paper.

Although the PLUTO code is a (kind of) public code, it requires a registration http://plutocode.ph.unito.it/download.html to get the source code. It is unclear to me whether it is okay to make ORION2 fully open-source given the policy adopted in PLUTO. If the ORION2 team already obtained the consent, it may also be good to be clarified.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/3771#issuecomment-963466862, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFACU6A2U3FF5GJ3F2DBTLULALRBANCNFSM5E3I6I3A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

changgoo commented 2 years ago

I've tested the code using a few examples included in the code. Given the complexity and multi-functionality of RMHD codes like this, I cannot check all functionalities and performances claimed. I simply checked off these given that there are published journal papers regarding each module (and extensive demonstrations in the document). The only remaining check is Automated tests.

Automated tests are extremely useful. I'm pretty sure that enabling a full regression suite for the code like this is not easy (almost impossible unless it is designed and developed in that way). However, it will still be possible to have a simple regression test suite for basic functionalities. For example, linear wave convergence tests can be added to verify basic functionality after the installation. I cannot recommend this as a mandatory requirement before the acceptance as it will still be a significant amount of work. But, it is highly recommended in the long run.

There are a few suggestions for the repository management. Again, these are not mandatory fixes for acceptance as it doesn't hurt the functionality of the code. But, this is recommended to make the code more user-friendly and accessible.

@dfm Other than these, I'm happy to sign off on the review.

dfm commented 2 years ago

👋 @zingale: I wanted to ping to make sure that this is still on your radar. I know that this is a busy time of year, but keep us posted and let me know if you run into any issues. Thanks!

zingale commented 2 years ago

yep, I'm on it. Should be done this week

dfm commented 2 years ago

Awesome - thank you!!

zingale commented 2 years ago

I have a question about the git repo on bitbucket. The name is "orion_release1", and looking at the git history, it seems like this repo was only created for the purposes of doing the JOSS paper.

So, is this the actual repo where the code is being developed? or is the development hidden from the public and there will be periodic drops of the code to new repos?

zingale commented 2 years ago

created docs issue: https://bitbucket.org/orionmhdteam/orion2_release1/issues/2/joss-review

zingale commented 2 years ago

are there any plans to host the docs on a webserver (readthedocs? or something else?). This would make it easier for users.

zingale commented 2 years ago

the orion-users group is private by default -- you should consider making the past posts public so new users can read past messages and understand the development community without having to ask permission to join.

zingale commented 2 years ago

created dependency issue https://bitbucket.org/orionmhdteam/orion2_release1/issues/3/joss-review-add-csh-as-dependency

zingale commented 2 years ago

I have not been able to build Chombo because of an interface mismatch: https://bitbucket.org/orionmhdteam/orion2_release1/issues/4/joss-review-chombo-compilation-error

zingale commented 2 years ago

I've tested the code using a few examples included in the code. Given the complexity and multi-functionality of RMHD codes like this, I cannot check all functionalities and performances claimed. I simply checked off these given that there are published journal papers regarding each module (and extensive demonstrations in the document). The only remaining check is Automated tests.

Automated tests are extremely useful. I'm pretty sure that enabling a full regression suite for the code like this is not easy (almost impossible unless it is designed and developed in that way). However, it will still be possible to have a simple regression test suite for basic functionalities. For example, linear wave convergence tests can be added to verify basic functionality after the installation. I cannot recommend this as a mandatory requirement before the acceptance as it will still be a significant amount of work. But, it is highly recommended in the long run.

There are a few suggestions for the repository management. Again, these are not mandatory fixes for acceptance as it doesn't hurt the functionality of the code. But, this is recommended to make the code more user-friendly and accessible.

  • After compiling the Chombo library, a huge number of untracked files appear, messing up git status output. I recommend to add some of rules in .gitignore (e.g., *_F.H).
  • There are many tracked files that are frequently overwritten under Test_Prob/ (e.g., some .F files). This makes it very difficult to see changes using git diff. I understand that it is recommended to use a separate directory for running actual problems. But, it is still recommended to remove unnecessary files from the repository.

@dfm Other than these, I'm happy to sign off on the review.

I would also like to understand the testing strategy.

zingale commented 2 years ago

The JOSS paper should address "State of the field: Do the authors describe how this software compares to other commonly-used packages?"

Can the author add a section explaining how Orion2 compares and differs from other open AMR MHD codes in terms of capability, performance, community, etc.

soffner commented 2 years ago

@zingale thank you for your comments and reviewing the code.

1.

I have a question about the git repo on bitbucket. The name is "orion_release1", and looking at the git history, it seems like this repo was only created for the purposes of doing the JOSS paper.

To some degree yes. ORION2 has some code (the FLD radiative transfer code that was developed at LNL), which is not allowed to be released (doesn't matter that other public FLD versions exist at this point). So we have to strip that out in order to release it.

2.

So, is this the actual repo where the code is being developed? or is the development hidden from the public and there will be periodic drops of the code to new repos?

We expect some development to take place in this repository. We also expect to migrate additional functionality from the private repository at a later date.

  1. We do not currently have plans to make a readthedocs page, but we agree that it would be easier than the current PDF documentation.

  2. Regarding the orion-groups. The group and ability to join was previously public. We have changed a setting that I think allows anyone to see google content. Admittedly there is not much there to see. We also have a private group-developers list, which we've been using up until now (all the users to date are also developers).

  3. Automated testing was a large topic of discussion over the years and we considered different options. However, we decided to stick with the individual, separate test problems, since implementation of automated testing that worked generally across different systems was complex.

    1. I have not been able to build Chombo because of an interface mismatch:

https://bitbucket.org/orionmhdteam/orion2_release1/issues/4/joss-review-chombo-compilation-error

created dependency issue https://bitbucket.org/orionmhdteam/orion2_release1/issues/4/joss-review-chombo-compilation-error https://bitbucket.org/orionmhdteam/orion2_release1/issues/3/joss-review-add-csh-as-dependency

We will update these tickets. Many of us compile Chombo with bash (i.e., without tcsh/csh). What is it you're seeing that suggests this needs to be a requirement?

Note that Chombo seriously needs to be updated to work with modern compilers. This is a major shortcoming of this library! The second issue is precisely caused by having too modern a compiler. After gcc10, stricter type-checking was employed, and the original authors of Chombo were quite sloppy in their implementation of the library. On the one hand, you might argue that the compiler is being too pedantic, i.e., that 0 is 0 for any plain-old data type. On the other hand, you might argue that the Chombo authors should have been more careful. This problem can be fixed in two ways: you can either add a compiler flag to convert this "error" into a mere warning, since it isn't really that serious, or else you can update the "0" in the call to Lapack's dgemm to the more appropriate value for the expected data type, which is "0.d0" as in the first line of each error message. The best way to do that is to replace "0" with "zero", which will get replaced during the pre-processing step.

  1. We will update the JOSS paper to address state-of-the-field.

—

You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/3771#issuecomment-981165812, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFACU3JKZHXM5LBIFT232LUOKVVBANCNFSM5E3I6I3A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

zingale commented 2 years ago

okay, I did a PR to fix the GCC 11 compilation issues: https://bitbucket.org/orionmhdteam/orion2_release1/pull-requests/2

zingale commented 2 years ago

I understand the issue of export control with rad transport (we had a similar experience), so that explanation is good by me.

zingale commented 2 years ago

I am going to defer to @dfm about the testing requirement for JOSS

zingale commented 2 years ago

The JOSS checklist asks to verify performance claims made in the paper. The paper says:

 The MHD code with AMR scales efficiently to many tens of thousands of cores.

I am not going to do a scaling test, but you should provide a reference for this statement.

dfm commented 2 years ago

I am going to defer to @dfm about the testing requirement for JOSS

We encourage, but don't require automated testing. There should at least be some clearly documented example scripts with the expected output clearly and quantitatively defined.

zingale commented 2 years ago

okay thanks @dfm. I think then we should just ask that a manual way to compare a run against expected output in some sense is provided.

soffner commented 2 years ago

In that case, I think what we have in the release meets the requirements -- many of the test problems have python scripts that compare the output with an expected solution and generate a fail message if agreement fails.

On Sat, Dec 4, 2021 at 11:05 AM Michael Zingale @.***> wrote:

okay thanks @dfm https://github.com/dfm. I think then we should just ask that a manual way to compare a run against expected output in some sense is provided.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/3771#issuecomment-986059313, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFACUYWLHNMDGYDWBN6DA3UPJC7DANCNFSM5E3I6I3A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

soffner commented 2 years ago

@whedon generate pdf

On Mon, Sep 27, 2021 at 2:22 PM whedon @.***> wrote:

Submitting author: @soffner https://github.com/soffner (Stella Offner http://orcid.org/0000-0003-1252-9916) Repository: https://bitbucket.org/orionmhdteam/orion2_release1/src/master/ https://bitbucket.org/orionmhdteam/orion2_release1/src/master/ Version: v1.0.0 Editor: @dfm https://github.com/dfm Reviewer: @zingale https://github.com/zingale, @changgoo https://github.com/changgoo Archive: Pending

⚠️ JOSS reduced service mode ⚠️

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 https://blog.joss.theoj.org/2020/05/reopening-joss. Status

[image: status] https://joss.theoj.org/papers/c8c824a316e16f6929ad490ee93d9f5d

Status badge code:

HTML:

Markdown: status

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository https://bitbucket.org/orionmhdteam/orion2_release1/src/master/ 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

@zingale https://github.com/zingale & @changgoo https://github.com/changgoo, 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 @dfm https://github.com/dfm 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 @zingale https://github.com/zingale

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission https://bitbucket.org/orionmhdteam/orion2_release1/src/master/. ✨ Conflict of interest

Code of Conduct

General checks

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • 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

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
  • 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

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • 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 and who the target audience is?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • 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 https://rmarkdown.rstudio.com/authoring_bibliographies_and_citations.html#citation_syntax ?

Review checklist for @changgoo https://github.com/changgoo

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission https://bitbucket.org/orionmhdteam/orion2_release1/src/master/. ✨ Conflict of interest

Code of Conduct

General checks

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • 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

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
  • 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

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • 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 and who the target audience is?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • 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 https://rmarkdown.rstudio.com/authoring_bibliographies_and_citations.html#citation_syntax ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/3771, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFACU3UFUMNO7RDVBWTM23UEC757ANCNFSM5E3I6I3A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

whedon commented 2 years ago

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

soffner commented 2 years ago

I believe we have addressed the last two issues raised by @zingale:

Thanks!

On Sun, Nov 28, 2021 at 5:10 PM Michael Zingale @.***> wrote:

The JOSS paper should address "State of the field: Do the authors describe how this software compares to other commonly-used packages?"

Can the author add a section explaining how Orion2 compares and differs from other open AMR MHD codes in terms of capability, performance, community, etc.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/3771#issuecomment-981170509, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFACU3CV3WER2VI647DO63UOKZEZANCNFSM5E3I6I3A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

zingale commented 2 years ago

thanks. I'm happy with things now. Thanks for merging my PRs.

zingale commented 2 years ago

@dfm I am happy with this and checked all my boxes.