openjournals / joss-reviews

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

[REVIEW]: FIRESONG: A python package to simulate populations of extragalactic neutrino sources #3194

Closed whedon closed 3 years ago

whedon commented 3 years ago

Submitting author: @ChrisCFTung (Chun Fai Tung) Repository: https://github.com/icecube/FIRESONG Version: 1.8 Editor: @eloisabentivegna Reviewer: @JostMigenda, @rafaelab Archive: 10.5281/zenodo.4747672

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

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

@JostMigenda & @rafaelab, 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 @eloisabentivegna 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 @JostMigenda

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @rafaelab

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 3 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @JostMigenda, @rafaelab 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 3 years ago
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.52 s (64.0 files/s, 22191.3 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
HTML                             8            729              0           6714
Python                          16            499            962           1375
Markdown                         4             55              0            278
TeX                              1             18              0            239
Jupyter Notebook                 2              0            356            162
YAML                             2              2              4             42
-------------------------------------------------------------------------------
SUM:                            33           1303           1322           8810
-------------------------------------------------------------------------------

Statistical information for the repository '771c9b8052973d42ee3f2975' was
gathered on 2021/04/20.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Alex Pizzuto                    29          1291            319           13.88
ChrisCFTung                      3           113             32            1.25
Chun Fai Tung                   58          1111            267           11.88
Ignacio Taboada                 73          1681           2002           31.76
Konstancja Satalecka             3           210             11            1.91
Michael Larson                   6            42             26            0.59
René Reimann                    71          2876           1615           38.73

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
Alex Pizzuto               1139           88.2          1.9               12.20
ChrisCFTung                 112           99.1          2.6               13.39
Chun Fai Tung               227           20.4         34.9               14.54
Ignacio Taboada              86            5.1         23.4               12.79
Michael J Larson             35          100.0          1.8                5.71
René Reimann               1237           43.0         37.4               10.02
whedon commented 3 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 3 years ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.3847/1538-4357/abcaa4 is OK
- 10.1103/PhysRevLett.124.051103 is OK
- 10.1140/epjc/s10052-019-6680-0 is OK
- 10.1126/science.aat1378 is OK
- 10.1126/science.aat2890 is OK
- 10.3847/1538-4357/835/1/45 is OK
- 10.1088/2041-8205/805/1/L5 is OK
- 10.1103/PhysRevD.94.103006 is OK
- 10.1088/1742-6596/632/1/012039 is OK

MISSING DOIs

- 10.22323/1.358.1004 may be a valid DOI for title: Characterization of the Astrophysical Diffuse Neutrino Flux with IceCube High-Energy Starting Events
- 10.22323/1.358.1017 may be a valid DOI for title: Measurement of the diffuse astrophysical muon-neutrino spectrum with ten years of IceCube data
- 10.22323/1.358.1015 may be a valid DOI for title: First Double Cascade Tau Neutrino Candidates in IceCube and a New Measurement of the Flavor Composition
- 10.22323/1.301.0663 may be a valid DOI for title: Constrains on the extragalactic origin of IceCube’s neutrinos using HAWC
- 10.22323/1.358.1017 may be a valid DOI for title: Measurement of the diffuse astrophysical muon-neutrino spectrum with ten years of IceCube data

INVALID DOIs

- None
JostMigenda commented 3 years ago

Hi @ChrisCFTung, thanks for the submission! I’ll install & test the software in a couple of days; for now, just a few initial comments and questions, after looking at the repo and the paper:

ChrisCFTung commented 3 years ago

Hi @JostMigenda,

Thank you for your comments and questions. Firstly regarding Theo Glauch's contribution, the git history was lost during a migration from Georgia Institute of Technology's git repository to github. In particular, FluxPDF.py was contributed by Theo.

We have also addressed the other issues you mentioned.

Regarding the comments on the paper:

  1. Description of FIRESONG's usage has been added in the Statement of Need section.
  2. Added refereces for Numpy and Scipy
  3. Added description for LEGEND in USAGE, to specify that LEGEND should be used when users want to simulate sources with a distribution that depends on luminosity.

Regarding init.py and setup.py, version string has been updated.

Regarding the README file, Python3 is now specified in Set-Up.

rafaelab commented 3 years ago

Hi @ChrisCFTung,

I have read the paper and I am now done testing the software. It looks fine overall, easy to install and easy to use. The examples are very helpful. I will divide my comments in two parts: first about the paper, second about the code.

--> PAPER

Overall I find the paper well-written and I have only minor comments.

--> CODE

I followed the tutorials and looked into the code to try it out. I had no problems installing nor running it. My comments are more about things that could be useful and some minor suggestions.

JostMigenda commented 3 years ago

I have now finished testing the software and have opened issues on the FIRESONG repository where appropriate. These are mostly minor points that shouldn’t take long to resolve, so I hope that we will reach acceptance of this paper before too long.

Apart from the noted issues, the code works well and seems both versatile and useful. It is tested and documented thoroughly. The notebooks give good examples for how to use the code and plot the results; they also nicely illustrate how Firesong and FluxPDF relate to each other. (Like Rafael, I would welcome it if you could add some examples for how to use Legend, too.)

One final point that’s missing right now—as far as I can see— is this one:

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

Just referring people to PRs/Issues on GitHub is perfectly fine, but this should be stated explicitly somewhere; probably in the README file.

Related to community guidelines: I see that the IceCube collaboration has a code of conduct (see Appendix F). Since the FIRESONG repository is under the IceCube organisation on GitHub, I’d assume that it applies to all interactions in this repo as well? If so, that would be good to mention as well; right next to the contribution guidelines.

itaboada commented 3 years ago

Interesting! The governance document, including the code of conduct (appendix F) are visible publicly! It's not like that we (IceCube) were hiding the document ... I will need to consult with others if the code of conduct, which was developed for IceCube internal affairs, can or should be applied to non-IceCube people.

JostMigenda commented 3 years ago

Yes—it's linked from IceCube's public web page at https://icecube.wisc.edu/collaboration/institutions/

I noticed that the "Scope" section of the CoC already mentioned the case of "an event participant that is not an IceCube collaborator"; that's why it seemed plausible to me that it could apply here. I understand that this is not what the CoC was originally designed for; on the other hand, having a separate CoC and separate reporting structures may introduce unnecessary confusion, especially since any parties to potential conflicts may likely be IceCube members?

Either way—thanks @itaboada for starting that discussion internally! As far as I understand (@eloisabentivegna, correct me if I'm wrong!), JOSS doesn't require a CoC so while it would be good to clarify this, it shouldn't stop us from proceeding with the review.

whedon commented 3 years ago

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

whedon commented 3 years ago

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

ChrisCFTung commented 3 years ago

Thank you for the comment @rafaelab .

Regarding your comment on the paper, in the paper, we have mentioned publications which applied Firesong on studies of gamma rays in the statement of need section of the paper. Indeed, providing a function to convert Firesong's output to other messengers is one of the open issue for Firesong (#45). However, before that service is mature, we don't want to propose a "standard" way of conversion to the users in this paper, because such conversion is heavily model dependent. We are leaving this up to the users' own capabilities for now.

Regarding the comments for the code

  1. the options will not be changed for the moment, due to possible breakage of existing scripts. However, we will look for a safe way to introduce more coherent option in the future.

  2. Added RA to the output, which is sampled from a uniform distribution. Since we expect the extragalactic sources to be isotropic

  3. Added a notebook as example for Legend.

rafaelab commented 3 years ago

Hi @ChrisCFTung,

  1. the options will not be changed for the moment, due to possible breakage of existing scripts. However, we will look for a safe way to introduce more coherent option in the future.

I completely understand that this breaks previous scripts. I saw @JostMigenda's issue and he recommended what I was going to suggest: that these conventions are kept in mind for future releases, if there are any.

  1. Added RA to the output, which is sampled from a uniform distribution. Since we expect the extragalactic sources to be isotropic

I am aware this is fairly simple, but I think it makes the code more complete and enables easy manipulations of the results as a dictionary.

  1. Added a notebook as example for Legend.

Having successfully tested this, I have no further comments and I am happy to recommend the paper for publication.

JostMigenda commented 3 years ago

All my comments and issues have been addressed and I'm happy to recommend this paper for publication.

Thanks to @ChrisCFTung and @itaboada for the prompt and helpful responses during the review and congratulations to you and your co-authors!

itaboada commented 3 years ago

Thank you @JostMigenda, @rafaelab

eloisabentivegna commented 3 years ago

(@eloisabentivegna, correct me if I'm wrong!), JOSS doesn't require a CoC so while it would be good to clarify this, it shouldn't stop us from proceeding with the review.

You are correct, @JostMigenda.

eloisabentivegna commented 3 years ago

Thanks @JostMigenda and @rafaelab for your prompt and thorough reviews. Loads of good points raised and improved. Thanks also to @ChrisCFTung and @itaboada for being so responsive.

I am happy to proceed with prepublication. @ChrisCFTung, could you generate a tagged version and submission archive, as per https://joss.readthedocs.io/en/latest/submitting.html#the-review-process? Please post the details here once you are done.

ChrisCFTung commented 3 years ago

Hi @eloisabentivegna I have tagged version 1.8 to be the submission version and uploaded the archive to zenodo. The doi of the submission is: 10.5281/zenodo.4744897 It is accessible at https://zenodo.org/record/4744897#.YJiq_GZKjOQ.

Thank you.

eloisabentivegna commented 3 years ago

@whedon set 1.8 as version

whedon commented 3 years ago

OK. 1.8 is the version.

eloisabentivegna commented 3 years ago

@whedon set 10.5281/zenodo.4744897 as archive

whedon commented 3 years ago

OK. 10.5281/zenodo.4744897 is the archive.

eloisabentivegna commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

eloisabentivegna commented 3 years ago

@whedon check references

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

OK DOIs

- 10.3847/1538-4357/abcaa4 is OK
- 10.1103/PhysRevLett.125.121104 is OK
- 10.1103/PhysRevLett.124.051103 is OK
- 10.22323/1.358.1004 is OK
- 10.22323/1.358.1017 is OK
- 10.22323/1.358.1015 is OK
- 10.1140/epjc/s10052-019-6680-0 is OK
- 10.1126/science.aat1378 is OK
- 10.1126/science.aat2890 is OK
- 10.3847/1538-4357/835/1/45 is OK
- 10.22323/1.301.0663 is OK
- 10.22323/1.358.1017 is OK
- 10.1088/2041-8205/805/1/L5 is OK
- 10.1103/PhysRevD.94.103006 is OK
- 10.1088/1742-6596/632/1/012039 is OK
- 10.1038/s41586-020-2649-2 is OK
- 10.1038/s41592-019-0686-2 is OK

MISSING DOIs

- None

INVALID DOIs

- None
eloisabentivegna commented 3 years ago

@ChrisCFTung, it all seems in order, except for two typos in the manuscript:

1) Line 43: "a inverse" -> "an inverse" 2) Line 77: "source distribution" -> "the source distribution"

Could you fix these, regenerate the archive, and post a line once you're done?

ChrisCFTung commented 3 years ago

@eloisabentivegna

Thank you for spotting the typos. I have updated the archive, and the new DOI is 10.5281/zenodo.4747672

eloisabentivegna commented 3 years ago

@whedon set 10.5281/zenodo.4747672 as archive

whedon commented 3 years ago

OK. 10.5281/zenodo.4747672 is the archive.

eloisabentivegna commented 3 years ago

@whedon accept

whedon commented 3 years ago
Attempting dry run of processing paper acceptance...
whedon commented 3 years ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.3847/1538-4357/abcaa4 is OK
- 10.1103/PhysRevLett.125.121104 is OK
- 10.1103/PhysRevLett.124.051103 is OK
- 10.22323/1.358.1004 is OK
- 10.22323/1.358.1017 is OK
- 10.22323/1.358.1015 is OK
- 10.1140/epjc/s10052-019-6680-0 is OK
- 10.1126/science.aat1378 is OK
- 10.1126/science.aat2890 is OK
- 10.3847/1538-4357/835/1/45 is OK
- 10.22323/1.301.0663 is OK
- 10.22323/1.358.1017 is OK
- 10.1088/2041-8205/805/1/L5 is OK
- 10.1103/PhysRevD.94.103006 is OK
- 10.1088/1742-6596/632/1/012039 is OK
- 10.1038/s41586-020-2649-2 is OK
- 10.1038/s41592-019-0686-2 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 3 years ago

:wave: @openjournals/joss-eics, this paper is ready to be accepted and published.

Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/2297

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/2297, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true
eloisabentivegna commented 3 years ago

Congratulations to all authors! 🎉

And, of course, thanks to @JostMigenda and @rafaelab for your time and help with the submission. Such an expedited review would have not been possible without your hard work.

danielskatz commented 3 years ago

@ChrisCFTung - As the AEiC this week, I've proofread your submission in preparation for publication, and found some minor issues, in https://github.com/icecube/FIRESONG/pull/67 - Please either merge this or let me know what you disagree with, then we can proceed to final publication.

ChrisCFTung commented 3 years ago

@danielskatz Thank you, I have merged the pull request.

danielskatz commented 3 years ago

@whedon accept

whedon commented 3 years ago
Attempting dry run of processing paper acceptance...
whedon commented 3 years ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.3847/1538-4357/abcaa4 is OK
- 10.1103/PhysRevLett.125.121104 is OK
- 10.1103/PhysRevLett.124.051103 is OK
- 10.22323/1.358.1004 is OK
- 10.22323/1.358.1017 is OK
- 10.22323/1.358.1015 is OK
- 10.1140/epjc/s10052-019-6680-0 is OK
- 10.1126/science.aat1378 is OK
- 10.1126/science.aat2890 is OK
- 10.3847/1538-4357/835/1/45 is OK
- 10.22323/1.301.0663 is OK
- 10.22323/1.358.1017 is OK
- 10.1088/2041-8205/805/1/L5 is OK
- 10.1103/PhysRevD.94.103006 is OK
- 10.1088/1742-6596/632/1/012039 is OK
- 10.1038/s41586-020-2649-2 is OK
- 10.1038/s41592-019-0686-2 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 3 years ago

:wave: @openjournals/joss-eics, this paper is ready to be accepted and published.

Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/2298

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/2298, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true
danielskatz commented 3 years ago

@whedon accept deposit=true

whedon commented 3 years ago
Doing it live! Attempting automated processing of paper acceptance...
whedon commented 3 years ago

🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦

whedon commented 3 years ago

🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨

Here's what you must now do:

  1. Check final PDF and Crossref metadata that was deposited :point_right: https://github.com/openjournals/joss-papers/pull/2299
  2. Wait a couple of minutes, then verify that the paper DOI resolves https://doi.org/10.21105/joss.03194
  3. If everything looks good, then close this review issue.
  4. Party like you just published a paper! 🎉🌈🦄💃👻🤘

    Any issues? Notify your editorial technical team...

danielskatz commented 3 years ago

Congratulations to @ChrisCFTung (Chun Fai Tung) and co-authors!!

And thanks to @JostMigenda and @rafaelab for reviewing, and @eloisabentivegna for editing!

whedon commented 3 years ago

:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](https://joss.theoj.org/papers/10.21105/joss.03194/status.svg)](https://doi.org/10.21105/joss.03194)

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.03194">
  <img src="https://joss.theoj.org/papers/10.21105/joss.03194/status.svg" alt="DOI badge" >
</a>

reStructuredText:
.. image:: https://joss.theoj.org/papers/10.21105/joss.03194/status.svg
   :target: https://doi.org/10.21105/joss.03194

This is how it will look in your documentation:

DOI

We need your help!

Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:

ChrisCFTung commented 3 years ago

@eloisabentivegna @JostMigenda @rafaelab @danielskatz Thank you, you guys have been great reviewers! I really appreciate this experience.