openjournals / joss-reviews

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

[REVIEW]: UncertainSCI: A Python Package for Noninvasive Parametric Uncertainty Quantification of Simulation Pipelines #4249

Closed editorialbot closed 1 year ago

editorialbot commented 2 years ago

Submitting author: !--author-handle-->@jessdtate<!--end-author-handle-- (Jess Tate) Repository: https://github.com/SCIInstitute/UncertainSCI Branch with paper.md (empty if default branch): Version: v1.0 Editor: !--editor-->@kellyrowland<!--end-editor-- Reviewers: @shahmoradi, @RichardClayton, @kellyrowland Archive: 10.5281/zenodo.8226383

Status

status

Status badge code:

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

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

@shahmoradi & @RichardClayton, 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 @kellyrowland 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 @shahmoradi

📝 Checklist for @RichardClayton

📝 Checklist for @kellyrowland

editorialbot commented 2 years 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 2 years ago
Software report:

github.com/AlDanial/cloc v 1.88  T=0.14 s (656.3 files/s, 86275.8 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          51           2503           2383           4928
Markdown                        13            311              0            644
TeX                              2             21              0            217
YAML                             3              8              8             97
reStructuredText                11            113            151             87
Ruby                             1             20              6             83
DOS Batch                        1              8              1             26
CSS                              1              7              0             23
JavaScript                       2              4              0             17
make                             1              4              7              9
Bourne Shell                     2              1              0              7
HTML                             1              0              0              5
-------------------------------------------------------------------------------
SUM:                            89           3000           2556           6143
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 2 years ago

Wordcount for paper.md is 1182

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

OK DOIs

- 10.1002/cnm.3395 is OK
- 10.1007/s10439-011-0391-5 is OK
- 10.1109/iembs.2005.1615736 is OK
- 10.1007/978-3-030-78710-3_49 is OK
- 10.22489/CinC.2020.275 is OK
- 10.23919/cinc53138.2021.9662950 is OK
- 10.23919/cinc53138.2021.9662837 is OK
- 10.1016/j.brs.2021.10.226 is OK
- 10.1137/17M1140960 is OK
- 10.5802/smai-jcm.24 is OK
- 10.1553/etna_vol50s71 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot 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:

kellyrowland commented 2 years ago

Hi @shahmoradi & @RichardClayton - just a reminder ping to start on your reviews when you can. Please feel free to @ me if you have any questions or run into any issues!

kellyrowland commented 2 years ago

Hi @RichardClayton @shahmoradi - a reminder to start on this review when you are able to. If you're unable to conduct the review, please let me know so that I can find another reviewer.

RichardClayton commented 2 years ago

Review checklist for @RichardClayton

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

shahmoradi commented 2 years ago

How can I generate the checklist?

danielskatz commented 2 years ago

You need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

shahmoradi commented 2 years ago

Review checklist for @shahmoradi

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

kellyrowland commented 2 years ago

Hi @shahmoradi @RichardClayton thanks for getting started on your reviews - just checking in; please let me know if you have any questions or concerns.

RichardClayton commented 2 years ago

Hi Kelly,

Thanks -- I am just recovering from an unpleasant bout of covid, which took me out for 2 weeks. I am just catching up with emails and all of the other tasks that have been piling up.

I did have a question about feedback to the authors. Is there a mechanism to provide comments on the manuscript beyond the tickboxes on the checklist?

Best wishes,

Richard

On Mon, 2 May 2022 at 17:58, Kelly L. Rowland @.***> wrote:

Hi @shahmoradi https://github.com/shahmoradi @RichardClayton https://github.com/RichardClayton thanks for getting started on your reviews - just checking in; please let me know if you have any questions or concerns.

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4249#issuecomment-1115121970, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBW7NA4OGEEOJ7GSR33ORLVIACTZANCNFSM5Q7NDIRQ . You are receiving this because you were mentioned.Message ID: @.***>

-- Professor Richard H Clayton INSIGNEO Institute for In-Silico Medicine and Department of Computer Science, University of Sheffield.

kellyrowland commented 2 years ago

Hi @RichardClayton - I'm really sorry to hear that, and I hope your recovery is progressing along as well as it can.

For more lengthy suggestions or requests, we suggest that reviewers open issues in the code's Github repository. So, you'll want to navigate to the issues page at https://github.com/SCIInstitute/UncertainSCI/issues and create new issues there for suggestions or requests. It can be helpful to link to those issues here in this review issue as well, to keep track of things.

RichardClayton commented 2 years ago

Apologies that I have taken so long to complete my initial review.

I am content to check off most of the boxes for my review. However, there are some minor revisions that I would suggest. These are:

Functionality documentation - The documentation at https://uncertainsci.readthedocs.io/en/latest/api_docs/index.html is of good quality, and there are some nice demos and tutorials, which enable the user to understand how the API works. However I would like to have seen a more comprehensive documentation of the different functions, so that users can quickly find what they are looking for without trawling through the source code.

State of the field - The paper is well written, and commendably short. However, I would like to have seen some reference to other methods for UQ aside from polynomial chaos expansions.

Either in the paper or in the documentation, it would also be very helpful to include a worked example or demo of a non-intrusive UQ exercise with an existing simulator that is not built in to UncertainSCI -- maybe by expanding Figure 1 in the paper. My guess is that most users will wish to deploy UncertainSCI on this type of code, and so this type of example would be expected to improve the use experience.

Best wishes,

Richard

kellyrowland commented 2 years ago

@RichardClayton thank you for the thorough review! I understand that just about everyone is stretched quite thin these days, and the JOSS team really appreciates the effort of reviewers.

@shahmoradi checking in again on your review status - please let me know about an update when you can. If you need to set down the review for any reason, please just let me know that so I can find another reviewer.

shahmoradi commented 2 years ago

Apologies for my delayed response. The mentions of this thread has apparently been mixed and overlooked among many other daily github emails.

I have left the three relevant items in the checklist unmarked for now until I receive the authors' response.

Sincerely

kellyrowland commented 2 years ago

hi @jessdtate 👋 checking in on any updates toward review completion if you have them.

jessdtate commented 2 years ago

Thanks for checking up @kellyrowland. We are working on implementing the changes suggested by the reviewers.

Do we need to worry about paper length if we add a little more on other methods and software?

kellyrowland commented 2 years ago

Thanks for the update - the suggested mark is around 1000 words, but that's flexible. Adding more material to round out the paper should be fine, but I'd be a little wary of, say, anything approaching 1500 words.

kellyrowland commented 2 years ago

Hi @openjournals/joss-eics @jessdtate @shahmoradi @RichardClayton - I just wanted to let you all know that I will be out of office and not checking emails, Github, etc. starting September 10 and will be returning on October 3. Happy to field any questions through the end of the week.

kellyrowland commented 2 years ago

Hi @jessdtate -checking in here to ask if you think the paper is ready for another look by the reviewers? I see some recent commits in the joss_feedback branch and just wanted to touch base.

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

kellyrowland commented 1 year ago

@jessdtate do you have any estimate of when the joss_feedback branch might get merged so that I can ping the reviewers here to pick this back up?

oliviaguest commented 1 year ago

@kellyrowland maybe email them? They seem to have forgotten to check their GitHub notifications, possibly? ☺️

kellyrowland commented 1 year ago

Hi @RichardClayton @shahmoradi 👋 it looks like some updates have recently come into the UncertainSCI project when you're able to take a look to pick the reviews back up here.

CCing @jessdtate in case they'd like to add anything else.

jessdtate commented 1 year ago

I've made a new tag for review: https://github.com/SCIInstitute/UncertainSCI/releases/tag/v1.0rc2 . Reviewers can use the master branch, the linked tag, or the equivalent Pypi version to test. Key changes include:

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

shahmoradi commented 1 year ago

@jessdtate Thank you for your efforts and revision. I attempted to run the tests last night, but finishing them took more than 30 minutes without any reports, which was longer than my patience. I ended up canceling it because the tests used up the CPU at a high clock rate for a long time.

  1. How long do these tests take to complete?
  2. Is there a coverage report for these tests to verify the comprehensiveness of the tests? Setting up an automated Code Coverage report generator would be an ideal solution.
jessdtate commented 1 year ago

The test shouldn't take much longer than that, but we are using a coverage test service: https://coveralls.io/github/SCIInstitute/UncertainSCI?branch=master There is a badge on the readme to showing the coverage too

kellyrowland commented 1 year ago

@RichardClayton please let me know if you're able to finish out your review in the near future or if I should try to find another reviewer to conduct a review.

kellyrowland commented 1 year ago

@shahmoradi would you be willing to rerun the tests (possibly in a background process) and see if they can complete in under an hour? I don't know that JOSS has any limit on how long tests might take to run and we'd appreciate your verification.

RichardClayton commented 1 year ago

@RichardClayton please let me know if you're able to finish out your review in the near future or if I should try to find another reviewer to conduct a review.

I'm able to complete my review this week -- sorry to have missed recent notifications. Essentially I am happy with the responses, but it would be useful to check that @shahmoradi can run the tests OK.

jessdtate commented 1 year ago

another note on the tests, they should only take a couple minutes to run. I've added them to the build test on github actions under Run tests with pytest. it takes about a minute on those resources, and it is setup for multiplatform. Here is a link to the latest run, which should be available for a few weeks.

shahmoradi commented 1 year ago

Please give me a day or two to address this review. We are in the finals week, which is among the busiest times of the year. Thank you.

On Wed, May 3, 2023, 2:03 PM Jess Tate @.***> wrote:

another note on the tests, they should only take a couple minutes to run. I've added them to the build test on github actions under Run tests with pytest. it takes about a minute on those resources, and it is setup for multiplatform. Here is a link to the latest run https://github.com/SCIInstitute/UncertainSCI/actions/runs/4875480673/jobs/8697731884#step:8:78, which should be available for a few weeks.

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4249#issuecomment-1533552397, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABDXNCJNAY6B3BFUJ6RX27TXEKTX7ANCNFSM5Q7NDIRQ . You are receiving this because you were mentioned.Message ID: @.***>

kellyrowland commented 1 year ago

Thanks for the notes and work here, everyone. Totally understood about finals week - I'll check back in a week or two to ping folks again if there hasn't been any further activity.

kellyrowland commented 1 year ago

@shahmoradi would you have a chance to try the tests again in the near future?

@RichardClayton please let me and/or the authors know if/when you have any updates on your review here

shahmoradi commented 1 year ago

Yes, I will finalize this review by June 2.

kellyrowland commented 1 year ago

@shahmoradi @RichardClayton checking in on this, please let me know when you have any updates.

shahmoradi commented 1 year ago

My apologies again for missing this review among many GitHub notifications. Reminders are always helpful, and thank you for reminding me. I have marked all items in the checklist as complete. Test coverage of 50%, as reflected in the repository badge, is quite a dismal coverage rate for a good package. Authors should strive to bring it above 90% to ensure quality and correctness. But I trust the developers' ability and commitment to do so without delaying the JOSS publication.

kellyrowland commented 1 year ago

Thank you very much for your time and effort in the review process here!

kellyrowland commented 1 year ago

In the interest of moving this to publication and because we are very close to having two complete reviews, I am going to add myself as a reviewer here to check and verify the few remaining items.

kellyrowland commented 1 year ago

@editorialbot add @kellyrowland as reviewer

editorialbot commented 1 year ago

@kellyrowland added to the reviewers list!

kellyrowland commented 1 year ago

Review checklist for @kellyrowland

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

kellyrowland commented 1 year ago

@jessdtate this looks good to me. I will note that the various links in the "Overview" section of the README are broken, but the standalone user documentation is quite comprehensive and I don't think it's a blocker for paper acceptance.

At this point I will ask you to issue a new tagged release of the software (if changed since the start of the review), and archive it (on Zenodo, figshare, or elsewhere). Then, please then post the version number and archive DOI here in the review issue, and I'll follow some subsequent wrap-up steps from there.

kellyrowland commented 1 year ago

hi @jessdtate pinging on this in case you're able to move forward with the wrap up steps:

At this point I will ask you to issue a new tagged release of the software (if changed since the start of the review), and archive it (on Zenodo, figshare, or elsewhere). Then, please then post the version number and archive DOI here in the review issue, and I'll follow some subsequent wrap-up steps from there.

jessdtate commented 1 year ago

Hi @kellyrowland, Thanks for the reminder. I've make a release ( v1.0 ) and DOI: 10.5281/zenodo.8226383. Let me know what next steps that you'd like me to do.