openjournals / joss-reviews

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

[REVIEW]: 'HallThruster.jl: a Julia package for 1D Hall thruster discharge simulation' #4672

Closed editorialbot closed 1 year ago

editorialbot commented 2 years ago

Submitting author: !--author-handle-->@archermarx<!--end-author-handle-- (Thomas Marks) Repository: https://github.com/UM-PEPL/HallThruster.jl Branch with paper.md (empty if default branch): Version: v0.8.3 Editor: !--editor-->@lucydot<!--end-editor-- Reviewers: @Rupali-Sahu, @TomGoffrey, @lucydot Archive: 10.5281/zenodo.8066246

Status

status

Status badge code:

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

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

@StanczakDominik & @Rupali-Sahu, 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 @lucydot 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 @Rupali-Sahu

šŸ“ Checklist for @TomGoffrey

šŸ“ Checklist for @lucydot

archermarx commented 1 year ago

Here are some other people i found in the JOSS list of reviewers that may be suitable

benjaminbolling rangsimanketkaew pkairys amorenobr p-ortmann TomGoffrey joglekara CFGrote sigvaldm rogeriojorge michaeltouati akaptano OsAmaro hasantahir

lucydot commented 1 year ago

@akaptano @hasantahir @OsAmaro - are any of you available to review this repo? Review has been underway for a while: one reviewer has finished but we really need another set of eyes on it. Please let me know if you have capacity to help!

hasantahir commented 1 year ago

Hi @lucydot

Sorry. I am starting teaching next week so can't commit. All the best to this repo!

lucydot commented 1 year ago

@sigvaldm, @rogeriojorge, @michaeltouati - are any of you available to review this repo? Review has been underway for a while: one reviewer has finished but we really need another set of eyes on it. Please let me know if you have capacity to help!

lucydot commented 1 year ago

@TomGoffrey @joglekara - are either of you available to review this repo? Review has been underway for a while: one reviewer has finished but we really need another set of eyes on it. Please let me know if you have capacity to help!

TomGoffrey commented 1 year ago

@lucydot I could probably take a look, but won't be able to do so for a few weeks. Would this work?

joglekara commented 1 year ago

Happy to take it on but Iā€™m away for a couple of weeks

On Sun, Mar 5, 2023 at 1:43 PM Tom Goffrey @.***> wrote:

@lucydot https://github.com/lucydot I could probably take a look, but won't be able to do so for a few weeks. Would this work?

ā€” Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4672#issuecomment-1455215442, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADGQZ7TVGYJHKBDICWLOUCLW2UCGHANCNFSM56I255IA . You are receiving this because you were mentioned.Message ID: @.***>

rogeriojorge commented 1 year ago

Hi,

Thank you for reaching out. Although the repository appears intriguing, unfortunately I am presently unavailable for a review. I anticipate having more availability in the future.

Best, Rogerio

On 28 Feb 2023, at 15:08, Lucy Whalley @.***> wrote:

@sigvaldm https://github.com/sigvaldm, @rogeriojorge https://github.com/rogeriojorge, @michaeltouati https://github.com/michaeltouati - are any of you available to review this repo? Review has been underway for a while: one reviewer has finished but we really need another set of eyes on it. Please let me know if you have capacity to help!

ā€” Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4672#issuecomment-1448351670, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABUAHSAKRVI7WJWBKJZ7JI3WZYIHBANCNFSM56I255IA. You are receiving this because you were mentioned.

lucydot commented 1 year ago

@TomGoffrey thank-you very much. Does a timeline where we aim to complete the review in 4-6 weeks work for you?

TomGoffrey commented 1 year ago

@lucydot Should be fine - I'm back at work late next week, but feel free to start the review process now.

lucydot commented 1 year ago

@TomGoffrey thats great. I'll add you as a reviewer. The review has already been underway for quite some time - however originally assigned reviewer is unavailable, so we need another set of eyes on it. Thank you for stepping in. The first step will be to generate a checklist with @editorialbot generate my checklist.

lucydot commented 1 year ago

@editorialbot add @TomGoffrey as reviewer

editorialbot commented 1 year ago

@TomGoffrey added to the reviewers list!

lucydot commented 1 year ago

@editorialbot remove @StanczakDominik as reviewer

editorialbot commented 1 year ago

@StanczakDominik removed from the reviewers list!

TomGoffrey commented 1 year ago

Review checklist for @TomGoffrey

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

lucydot commented 1 year ago

Hi @TomGoffrey a friendly reminder/prompt for your review āœ…

TomGoffrey commented 1 year ago

Hi @TomGoffrey a friendly reminder/prompt for your review āœ…

Sorry for the delay, managed to catch covid on my flight home so my return to work has been a bit delayed, getting started on the review today!

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

archermarx commented 1 year ago

Hi, just checking in to see if there have been any updates on this. Thanks!

lucydot commented 1 year ago

@TomGoffrey - just to echo @archermarx comment above, any updates on your review would be much appreciated.

lucydot commented 1 year ago

Hi @TomGoffrey I can see from tickboxes you are quite far through the review process āœ… - a prompt to please complete the review when possible and update us here.

lucydot commented 1 year ago

A note here that I have contacted @TomGoffrey via email - I know how easy it is to miss those github mentions!

archermarx commented 1 year ago

@TomGoffrey Another ping. Hopefully we can finish this up soon!

lucydot commented 1 year ago

Oh dear we really are having a tough time with this review process.

As it has already been signed off by one reviewer working in the field, and most of the review is complete by a second person in the field, I will ask the editorial team if I (or someone else in the editorial team) can complete the outstanding parts.

lucydot commented 1 year ago

In the meantime @TomGoffrey if you do have time to complete the review, that would be the preferred option.

lucydot commented 1 year ago

ok @archermarx I have been given the go-ahead to review myself, given that we have one person from the field that has already signed off, and one half-complete review.

I am a little snowed under with other review commitments at the moment, but I will aim to get this done in the next 2-3 weeks.

In the meantime @TomGoffrey if you are able to finish your review that would be the strongly preferred option.

archermarx commented 1 year ago

Thanks Lucy. Hopefully we can finally wrap this up.

lucydot commented 1 year ago

@editorialbot add @lucydot as reviewer

editorialbot commented 1 year ago

@lucydot added to the reviewers list!

lucydot commented 1 year ago

Review checklist for @lucydot

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

lucydot commented 1 year ago

@editorialbot generate pdf

lucydot 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.1063/5.0021474 is OK
- 10.2514/6.2001-3505 is OK
- 10.1088/1361-6595/ab0f70 is OK
- 10.1063/1.368529 is OK
- 10.1063/1.1371519 is OK
- 10.1088/0963-0252/14/4/011 is OK
- 10.1063/1.5055750 is OK
- 10.1002/9780470436448.ch7 is OK
- 10.1063/1.4972269 is OK
- 10.1145/3511528.3511535 is OK
- 10.1137/141000671 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:

lucydot commented 1 year ago

Hi @archermarx I've just started working through the checklist.

lucydot commented 1 year ago

Paper:

lucydot commented 1 year ago

Tutorials:

It may be that your system is setup to output plot as default. I am using a fresh install of Julia and IJulia. May catch others out so may be worth making a note of behaviour if don't want to add additional keyword argument to every plot call (which does feel clunky way to handle).

archermarx commented 1 year ago

Hi Lucy! Thanks for taking the time to review. I can respond to your comments here:

Paper:

* should `one-dimensional fluid code` be `one-dimensional fluid dynamics code`?

In this case, no. Fluid is used here in contrast to kinetic or particle-in-cell. To clarify, I have changed the sentence to "...one dimensional fluid Hall thruster code".

* you should be able to place the list of references on page 2 into a single bracket: `[@boeuf1998; @ahedo2001; @haraquasineutralfluid; @sahu_ffm; @mikellides_1D]`

* `.(Thomas, 1949)` --> ` (Thomas, 1949)`(additional period removed)

* The `Landmark 1D fluid test case` weblink is broken (in reference list). Please be sure to use correct [citation syntax](https://pandoc.org/MANUAL.html#extension-citations).

* I think Gowda et al has now been peer reviewed+published (currently referencing the arxiv paper)

Thanks for these notes on the references. I have updated the links.

archermarx commented 1 year ago

Hi @archermarx I've just started working through the checklist.

* I couldn't find guidelines for "Report issues or problems with the software" or "Seek support". This could be fixed with a couple of sentences on the "Home" page of your documentation, for example.

* Link to tutorial from the README is broken

* PR with very small update to install instruction here: [Update installation instructions UM-PEPL/HallThruster.jl#83](https://github.com/UM-PEPL/HallThruster.jl/pull/83) (tested on Julia 1.9)

Thanks for catching this! I've accepted your PR and made the relevant changes.

Tutorials:

* I had to add `show=true` when using `plot!` within a user-defined function. For example, in the tutorial function `plot_current`: `plot!(p, t_ms, ion_current, label = "Ion current (cathode)") --> `plot!(p, t_ms, ion_current, label = "Ion current (cathode)",show=true)` (would also need updating at other places).

It may be that your system is setup to output plot as default. I am using a fresh install of Julia and IJulia. May catch others out so may be worth making a note of behaviour if don't want to add additional keyword argument to every plot call (which does feel clunky way to handle).

* There are some warnings towards the end of the tutorial. Is this acceptable? May be worth making a note if so, or fix if not.

Thanks for this. I added a note about the warnings to the tutorial and changed the plot parts of the tutorial to explicitly call display.

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

archermarx 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.1063/5.0021474 is OK
- 10.2514/6.2001-3505 is OK
- 10.1088/1361-6595/ab0f70 is OK
- 10.1063/1.368529 is OK
- 10.1063/1.1371519 is OK
- 10.1088/0963-0252/14/4/011 is OK
- 10.1063/1.5055750 is OK
- 10.1002/9780470436448.ch7 is OK
- 10.1063/1.4972269 is OK
- 10.1145/3511528.3511535 is OK
- 10.1137/141000671 is OK

MISSING DOIs

- None

INVALID DOIs

- None
archermarx commented 1 year ago

Thanks editorialbot!

lucydot commented 1 year ago

Hi @archermarx - my previous PR missed an @ sign in the citation list, I think this caused it not to format correctly. Just put in a PR that fixes. Can you merge? https://github.com/UM-PEPL/HallThruster.jl/pull/84

lucydot commented 1 year ago

That is my review complete! Now back into editor-mode.

lucydot commented 1 year ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

lucydot commented 1 year ago

@archermarx please can you work through the checklist above? You won't be able to tick them off; just let me know when they are complete in this thread.