search

openjournals / joss-reviews

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

[PRE REVIEW]: SeisModels.jl: A Julia package for models of the Earth's interior #2020

Closed whedon closed 4 years ago

whedon commented 4 years ago

Submitting author: @anowacki (Andy Nowacki) Repository: https://github.com/anowacki/SeisModels.jl Version: v1.0.0 Editor: @kbarnhart Reviewers: @daniellivingston, @fhorrobin

Author instructions

Thanks for submitting your paper to JOSS @anowacki. Currently, there isn't an JOSS editor assigned to your paper.

@anowacki if you have any suggestions for potential reviewers then please mention them here in this thread (without tagging them with an @). In addition, this list of people have already agreed to review for JOSS and may be suitable for this submission (please start at the bottom of the list).

Editor instructions

The JOSS submission bot @whedon is here to help you find and assign reviewers and start the main review. To find out what @whedon can do for you type:

@whedon commands
whedon commented 4 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks.

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 4 years ago 
Software report (experimental):

github.com/AlDanial/cloc v 1.84  T=0.04 s (863.5 files/s, 60761.7 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Julia                           22            216            107           1509
Markdown                        10            127              0            510
YAML                             3             14              7             79
TeX                              1              7              0             71
TOML                             2              5              0             22
-------------------------------------------------------------------------------
SUM:                            38            369            114           2191
-------------------------------------------------------------------------------

Statistical information for the repository '2020' was gathered on 2020/01/17.
No commited files with the specified extensions were found.
kyleniemeyer commented 4 years ago

Hey @kbarnhart, could you edit this for us?

whedon commented 4 years ago 
Reference check summary:

OK DOIs

- 10.1111/j.1365-246X.1995.tb03540.x is OK
- 10.1111/j.1365-246X.1991.tb06724.x is OK
- 10.1016/0031-9201(81)90046-7 is OK
- 10.1126/science.1199375 is OK
- 10.1785/gssrl.70.2.154 is OK
- 10.1785/gssrl.81.3.530 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

anowacki commented 4 years ago

@anowacki if you have any suggestions for potential reviewers then please mention them here in this thread

Choosing from the list, I would suggest the following would be able to provide a review:

kbarnhart commented 4 years ago

@kyleniemeyer I can edit

kbarnhart commented 4 years ago

@whedon assign @kbarnhart as editor

whedon commented 4 years ago

OK, the editor is @kbarnhart

kbarnhart commented 4 years ago

@anowacki I have a couple of clarification questions:

The software has been benchmarked against independent implementations and has a significant speed advantage over similar Python software.

anowacki commented 4 years ago

Thank you for the questions, @kbarnhart.

  • Could you give me an example of using SeisModels.jl in a research application? I know the paper indicates that this package is used in research projects, but a specific example would be helpful.

The software has been used by a research group here at the University of Leeds to investigate 1D models of the Earth in our group meetings. Some of these studies are available at https://github.com/anowacki/DERG_PREM. There is also an article in review for inclusion in an AGU monograph (Nowacki & Cottaar, 2020) which has used SeisModels.

If you feel the manuscript itself should contain examples of usage or text similar to that above, I would be happy to include that.

  • The paper makes a performance claim but I don't see any demonstration of this claim (in the paper or the docs). Could you elaborate on this claim.

    The software has been benchmarked against independent implementations and has a significant speed advantage over similar Python software.

The benchmarking was done against a simple equivalent, and unpublished, Python code, which is available here: https://github.com/andreww/prem4derg. I am not aware of any other open-source and freely-available software apart from that which works similarly to SeisModels in calculating bulk Earth properties, which is why concrete performance benchmarks against other implementations aren't included.

Would it be better to remove the 'comparison' and simply state some times, such as: "For a PREM-like model, computation of the surface gravity takes 5 µs, whilst calculating pressure at the centre of the body takes 12 ms."

Note that the equivalent calculations for the Python code take respectively 560 µs and 720 ms.

References

kbarnhart commented 4 years ago

@anowacki thanks for the responses, they are very helpful.

A couple of comments from me in advance of review. My comments are mostly things that I anticipate would come up in review based on looking at the package and knowing the review process and criteria.

Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

kbarnhart commented 4 years ago

:wave: @daniellivingston @lheagy @fhorrobin would you be willing to review this submission for JOSS? We carry out our checklist-driven reviews here in GitHub issues and follow these guidelines: https://joss.readthedocs.io/en/latest/review_criteria.html

This is a pre-review issue that I use to coordinate reviewers. Once there are sufficient reviewers (usually 2-3), I will open a new issue where the review will take place.

If you are not able, please let me know if you have recommendations for other reviewers. Please recommend without mentioning a handle (e.g., you would refer to me as @ kbarnhart instead of @kbarnhart). This way we attempt to reduce the number of people who are subscribed to this thread.

anowacki commented 4 years ago

Thanks for those suggestions, @kbarnhart, and for moving things forward.

I have pushed an updated version of the manuscript to the repo's master branch with concrete example timings and instructions on how to test the package (https://github.com/anowacki/SeisModels.jl/commit/9e755d1225983960d47d10699fef76a73012a85a).

daniellivingston commented 4 years ago

Hi @kbarnhart , this is right up my alley. I'd be happy to review.

kbarnhart commented 4 years ago

Thanks for being willing to review @daniellivingston . I'll add you as a reviewer. Once I have a sufficient number of reviewers (typically a few days), I'll open a new issue where the review will take place.

kbarnhart commented 4 years ago

@whedon assign @daniellivingston as reviewer

whedon commented 4 years ago

OK, @daniellivingston is now a reviewer

kbarnhart commented 4 years ago

:wave: @lheagy @fhorrobin a friendly reminder to consider reviewing this submission to JOSS? We carry out our checklist-driven reviews here in GitHub issues and follow these guidelines: https://joss.readthedocs.io/en/latest/review_criteria.html

This is a pre-review issue that I use to coordinate reviewers. Once there are sufficient reviewers (usually 2-3), I will open a new issue where the review will take place.

If you are not able, please let me know if you have recommendations for other reviewers. Please recommend without mentioning a handle (e.g., you would refer to me as @ kbarnhart instead of @kbarnhart). This way we attempt to reduce the number of people who are subscribed to this thread.

fhorrobin commented 4 years ago

This is right up my alley as well, happy to assist.

kbarnhart commented 4 years ago

@whedon add @fhorrobin as reviewer

whedon commented 4 years ago

OK, @fhorrobin is now a reviewer

kbarnhart commented 4 years ago

Thanks for being willing to review this package @fhorrobin 👍 .

I'd like to find one more reviewer for this before I move it from the pre-review to the review stage.

:wave: @rowanc1 @leouieda would you be willing to review this submission for JOSS? We carry out our checklist-driven reviews here in GitHub issues and follow these guidelines: https://joss.readthedocs.io/en/latest/review_criteria.html

This is a pre-review issue that I use to coordinate reviewers. Once there are sufficient reviewers (usually 2-3), I will open a new issue where the review will take place.

If you are not able, please let me know if you have recommendations for other reviewers. Please recommend without mentioning a handle (e.g., you would refer to me as @ kbarnhart instead of @kbarnhart). This way we attempt to reduce the number of people who are subscribed to this thread.

kbarnhart commented 4 years ago

I'm going to move this forward from pre-review to review stage.

If possible, I'd like to add one more reviewer. @lheagy, @rowanc1, @leouieda if you are interested, able, or have another reviewer to recommend, please let me know.

kbarnhart commented 4 years ago

@whedon start review

whedon commented 4 years ago

OK, I've started the review over in https://github.com/openjournals/joss-reviews/issues/2043.

leouieda commented 4 years ago

Hi @kbarnhart sorry, I don't have the bandwidth for this right now. But I would recommend seisman or joa-quim.

kbarnhart commented 4 years ago

@leouieda thanks for the recommendations.

:wave: @seisman @joa-quim would you be willing to review this submission for JOSS? We carry out our checklist-driven reviews here in GitHub issues and follow these guidelines: https://joss.readthedocs.io/en/latest/review_criteria.html

This is a pre-review issue that I use to coordinate reviewers. If you are able, I will add you to the review issue at openjournals/joss-reviews/issues/2043.

seisman commented 4 years ago

Sorry, I'm not a Julia user.

joa-quim commented 4 years ago

Ok, I can do a review.

Joaquim

From: Katy Barnhart notifications@github.com Sent: Monday, February 3, 2020 3:37 PM To: openjournals/joss-reviews joss-reviews@noreply.github.com Cc: Joaquim Manuel Freire Luís jluis@ualg.pt; Mention mention@noreply.github.com Subject: Re: [openjournals/joss-reviews] [PRE REVIEW]: SeisModels.jl: A Julia package for models of the Earth's interior (#2020)

@leouiedahttps://github.com/leouieda thanks for the recommendations.

👋 @seismanhttps://github.com/seisman @joa-quimhttps://github.com/joa-quim would you be willing to review this submission for JOSS? We carry out our checklist-driven reviews here in GitHub issues and follow these guidelines: https://joss.readthedocs.io/en/latest/review_criteria.html

This is a pre-review issue that I use to coordinate reviewers. If you are able, I will add you to the review issue at /issues/2043https://github.com/openjournals/joss-reviews/issues/2043.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/openjournals/joss-reviews/issues/2020?email_source=notifications&email_token=AAEDF2NATO6ACWERAGJZZB3RBA223A5CNFSM4KICINL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKUJEWA#issuecomment-581472856, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAEDF2MM2AHUGNLMGXG2GP3RBA223ANCNFSM4KICINLQ.

kbarnhart commented 4 years ago

thanks @seisman and @joa-quim for the speedy responses. I'll add @joa-quim over on the review issue (#2043).