[REVIEW]: sntools: Event generator for supernova burst neutrinos

whedon commented 3 years ago

Submitting author: @JostMigenda (Jost Migenda) Repository: https://github.com/JostMigenda/sntools/ Version: v0.7.3 Editor: @xuanxu Reviewer: @bradkav, @benjaminrose Archive: 10.5281/zenodo.4688040

: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 badge code:

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

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

@bradkav & @benjaminrose, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

Make sure you're logged in to your GitHub account
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 @xuanxu 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 @bradkav

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the repository url?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@JostMigenda) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

Functionality

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

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

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] 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?

Review checklist for @benjaminrose

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the repository url?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@JostMigenda) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

Functionality

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

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

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] 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?

whedon commented 3 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @bradkav, @benjaminrose 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:

Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

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

PDF failed to compile for issue #2877 with the following error:

Can't find any papers to compile :-(

JostMigenda commented 3 years ago

@whedon generate pdf from branch joss-paper

whedon commented 3 years ago

Attempting PDF compilation from custom branch joss-paper. Reticulating splines etc...

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:

benjaminrose commented 3 years ago

SNtools review

This looks really useful! I see a few things that need a bit of an adjustment before I can recommend it to be accepted in JOSS. I know this list looks long, but I promise they are not too complicated and we should be able to converge on a acceptable submission quickly.

[x] sntools is a command line app. That needs to be more clear at the top to the documentation and readme.md
[x] Where is a readable version of the documentation? All I found was the latex source. There are ways to get Traves CI or GitHub actions to compile the source. This can be saved to its own branch if desired. Please link to a user facing documentation in either the Readme.md, the GitHub repo metadata, or both.
[x] Why does the first example command not work without optional arguments? I feel like you should show an example that can easily take simple tweaks.
[x] Why is this example passing the default to detector?
[x] This first example takes a while to run. Is there a shorter simulation that can be used just to test the installation process? Also, I had no idea how long this simulation was going to take, nor how far along it was while running. When running someone else's input file, it would be nice to get some output that says what it will do before it does it. Maybe add this to the verbose mode and add that to the first example?
[x] In the paper: "It is an event rate calculator—not an event generator—and states in its own documentation that it is “not intended to replace full detector simulations.” In contrast, sntools is intended for use in conjunction with a full detector simulation to perform more advanced, in-depth studies." -- in conjunction with is not the same as in place of. What exactly does sntools add to supplement to detector simulations that SNOwGLoBES can't? Is it the event level, as opposed to rate, information?
[x] Is there a walk through/tutorial for writing an input file? This is important since writing your own input file is the main thing a researcher will do.
[x] Tests don't install the package via pip. They currently test the source code, good. But don't test what will be sent installed from pypi, less good. This explains how you are testing your project directory and not how it will be installed by users, https://hynek.me/articles/testing-packaging/. This is a valid test suite, but may not be doing what you expect.

JostMigenda commented 3 years ago

Many thanks for these comments, @benjaminrose!

The first 5 ones are discussed in JostMigenda/sntools#16.

What exactly does sntools add to supplement to detector simulations that SNOwGLoBES can't? Is it the event level, as opposed to rate, information?

Exactly as you say—sntools generates individual events, whereas SNOwGLoBES only calculates the expected number of events in each energy bin.

Is there a walk through/tutorial for writing an input file? This is important since writing your own input file is the main thing a researcher will do.

I don’t think most users of sntools will write their own input files. Instead, input files will typically be generated by the groups that perform computer simulations of SNe and make the resulting outputs (neutrino fluxes, gravitational wave signals, …) available to the community; while typical users of sntools work on a specific neutrino experiment and take input files from an available simulation to generate event files with sntools and then run those events through their experiment’s own detector simulation & event reconstruction toolchain.

If a simulations group makes their neutrino fluxes available in a format that’s not yet supported by sntools, an sntools user may need to add support for that new input format. That’s why I’ve included brief instructions for how to do that in chapter 3 of the documentation.

Tests don't install the package via pip. They currently test the source code, good. But don't test what will be sent installed from pypi, less good.

I’ve opened JostMigenda/sntools#19 for this.

benjaminrose commented 3 years ago

Thanks @JostMigenda, these changes all look good. I now see where the documentation explains that input files come from simulations.

whedon commented 3 years ago

:wave: @benjaminrose, please update us on how your review is going.

whedon commented 3 years ago

:wave: @bradkav, please update us on how your review is going.

bradkav commented 3 years ago

Hi @JostMigenda, this looks like a really useful tool! The install proceeds smoothly and the examples look like they work pretty straightforwardly. I just have a few comments (most of which are simple/cosmetic) which will hopefully improve the accessibility for new users. My comments below are based on branch https://github.com/JostMigenda/sntools/tree/JostMigenda/issue16, which I think has already incorporated some changes suggested by @benjaminrose. (And sorry to drop this review on you just before Christmas.)

[x] Examples: After installing via pip, the files in the fluxes/ folder are not locally available, so the command line examples don't work (that is, the user would first have to download the flux examples manually). I would suggest adding an extra line of script in the README, such as
```
curl -L https://raw.githubusercontent.com/JostMigenda/sntools/master/fluxes/intp2001.data -o intp2001.data
```
which will then allow the user to run the examples in the README immediately after the pip install.
[x] Documentation: In the README you mention a number of input/output file formats (Princeton, NUANCE, RATPAC). I think it would be useful to have some links in the README to point the user to some specifications of these file formats. As pointed out by @benjaminrose, there should also be a link to the full documentation somewhere in the README.
[x] Tests: At the moment, it doesn't look like there are any tests that the user can perform to check that everything is working correctly (if they've installed via pip). One possibility is that you could include the expected output of the command line examples in the README, so that the user can manually check (e.g. using a specified random seed to guarantee reproducibility). Another would be automated tests which the user can run after installing.
[x] Community guidelines: I can't see any information in the README about suggestions for how to contribute or how to get help. It looks like the latex documentation has some of this information, so once there's a clear link to a readable version of this, I think this can be marked as resolved.

JostMigenda commented 3 years ago

Thanks for the comments @bradkav!

I spent the holiday period thinking about the first-run experience of new users; the result is in JostMigenda/sntools#20 and should cover your bullet points on examples and tests.

Regarding documentation: I’ve added a note at the beginning of the README file explaining where to find full documentation. The full documentation contains more details and/or links to official documentation for the input/output file formats. Let me know if there’s anything specific I’m missing!

Regarding community guidelines: I’ve added a brief section on issue reporting and contributing to the README, with an explicit pointer to the detailed instructions in the full documentation.

benjaminrose commented 3 years ago

@JostMigenda, I think this is really close. I see a few final tweaks that I think are needed before acceptance.

[x] Can you add a description of the available support options to the documentation? If it is just community (aka not lead by you) support centered around the GitHub issues, that is reasonable.
[x] I agree with @bradkav, please add a description on how to obtain the example fluxes files (link to the GitHub download or the curl example should be sufficient). I appear to have intuitively cloned the repo and missed this issue.
[x] I would also like to see you take another pass at clearly stating the statements of need, specifically sntools intended audience. This should be in the JOSS paper, and the documentation.

I think these are small issues and should lead to an acceptance shortly. Great work.

bradkav commented 3 years ago

Hi @JostMigenda! Thanks for making the latest changes. With https://github.com/JostMigenda/sntools/pull/20 and the latest updates to the readme/documentation, I don't think I have any further comments. Everything seems to run nicely now!

Once the remaining comments raised by @benjaminrose have been addressed, I am very happy to recommend this for publication!

JostMigenda commented 3 years ago

@bradkav Great; many thanks for your comments and suggestions!

@benjaminrose Quick replies to the three remaining comments:

I’ve added a sentence on getting support to the README file a few days ago in response to @bradkav, and have updated the documentation accordingly just now. (See the very start of chapter 3 in this compiled PDF.)
I added a setup function in JostMigenda/sntools#20, which downloads the sample flux file used in the examples. Can you please try this out (pip install -U sntools to get v0.7.1, then python -c 'import sntools; sntools.setup()') and let me know if you have any further suggestions?
I’ll try to look at the statement of need in the next few days.

benjaminrose commented 3 years ago

@whedon generate pdf from branch joss-paper

whedon commented 3 years ago

Attempting PDF compilation from custom branch joss-paper. Reticulating splines etc...

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:

benjaminrose commented 3 years ago

@JostMigenda, I think I was reviewing a slightly out of date version of the paper and project. It appears that all my concerns have been addressed.

benjaminrose commented 3 years ago

@xuanxu I recommend sntools for publication in JOSS. @JostMigenda, great work!

bradkav commented 3 years ago

@xuanxu I'm also now happy to recommend sntools for publication. Well done @JostMigenda!

xuanxu commented 3 years ago

@benjaminrose @bradkav: Great! Thank you both!

xuanxu commented 3 years ago

@whedon generate pdf from branch joss-paper

whedon commented 3 years ago

Attempting PDF compilation from custom branch joss-paper. Reticulating splines etc...

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:

xuanxu commented 3 years ago

@whedon check references from branch joss-paper

whedon commented 3 years ago

Attempting to check references... from custom branch joss-paper

whedon commented 3 years ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1140/epjc/s10052-020-7977-8 is OK
- 10.1016/S0370-2693(98)00087-2 is OK
- 10.1016/0370-1573(79)90010-3 is OK
- 10.1016/S0375-9474(99)00152-9 is OK
- 10.1103/PhysRevD.66.013007 is OK
- 10.1093/mnras/sty2164 is OK
- 10.1103/PhysRevD.78.085012 is OK
- 10.1103/PhysRevD.98.103020 is OK
- 10.1103/PhysRevD.89.093001 is OK
- 10.1016/j.nuclphysb.2016.02.012 is OK
- 10.1103/PhysRevLett.103.051105 is OK
- 10.1103/PhysRevLett.118.021101 is OK
- 10.1103/PhysRevLett.106.091101 is OK
- 10.1103/PhysRevLett.104.191102 is OK
- 10.1103/PhysRevD.74.105014 is OK
- 10.1103/PhysRevLett.97.241101 is OK
- 10.1088/1475-7516/2005/04/002 is OK
- 10.1103/PhysRevD.62.033007 is OK
- 10.1103/PhysRevD.68.093013 is OK
- 10.1093/ptep/pty134 is OK
- 10.1103/PhysRevC.98.034613 is OK
- 10.1103/PhysRevD.51.6146 is OK
- 10.1016/0370-2693(71)90050-5 is OK
- 10.1016/j.astropartphys.2016.04.003 is OK
- 10.1103/PhysRevC.67.035502 is OK
- 10.1088/0067-0049/205/1/2 is OK
- 10.1109/MCSE.2011.37 is OK

MISSING DOIs

- 10.1103/physrevd.86.125031 may be a valid DOI for title: High-resolution supernova neutrino spectra represented by a simple fit
- 10.1086/375130 may be a valid DOI for title: Monte Carlo Study of Supernova Neutrino Spectra Formation
- 10.1088/1475-7516/2006/05/012 may be a valid DOI for title: Earth matter effects in supernova neutrinos: optimal detector locations
- 10.1088/1475-7516/2003/06/006 may be a valid DOI for title: Identifying Earth matter effects on supernova neutrinos at a single detector
- 10.1016/s0370-2693(03)00616-6 may be a valid DOI for title: Precise quasielastic neutrino/nucleon cross-section

INVALID DOIs

- 10.1103/PhysRevD.76.081301, 10.1103/PhysRevD.77.029903 is INVALID
- https://doi.org/10.1038/s41592-019-0686-2 is INVALID because of 'https://doi.org/' prefix

xuanxu commented 3 years ago

@JostMigenda Whedon reports missing and invalid DOIs for some references but I think they are not used in the paper and can be removed from the .bib file so that should not be an issue.

xuanxu commented 3 years ago

@JostMigenda, everything looks good, you can take a final look at the pdf draft and if you find it OK, here are the next steps:

Please release a new tagged version from the current master so it includes all the changes made during the review process
Then archive that latest release in Zenodo (or a similar archival service)
Check the Zenodo deposit has the correct metadata: title and author names should match the paper; you may also add their ORCID info.

Once you do that please report here the version number and archive DOI

JostMigenda commented 3 years ago

Many thanks @benjaminrose and @bradkav for taking the time to review and giving me so many helpful comments!

@xuanxu I have submitted the corresponding physics-focussed paper to AAS yesterday. (@augustfly: manuscript number is AAS29735, in case you need that.) Should I tag & archive the latest release now? Or should we pause this review and do the final steps once the AAS paper is ready?

augustfly commented 3 years ago

@JostMigenda thanks for ID. yes, I've double-checked that this is being tracked appropriately on the AAS side.

xuanxu commented 3 years ago

@JostMigenda I'm going to pause this until the AAS paper has a DOI. After that you can update the paper.md file with the AAS journal name and DOI, and then tag & archive a new release.

Please report back here with the AAS paper's DOI, the new version number and the Zenodo (or similar) DOI.

For reference, the DOI of this paper (when it is published) will be 10.21105/joss.2877. Probably the AAS folks will want that.

JostMigenda commented 3 years ago

Just to give a quick update: The physics paper was just accepted by ApJ; I expect we’ll hear from their editorial office soon and can then progress here.

arfon commented 3 years ago

@JostMigenda - I've just head from the AAS that the DOI for their paper is 10.3847/1538-4357/abf7c4. I've opened a PR to add this to your paper (https://github.com/JostMigenda/sntools/pull/24), but I'm not sure if the journal name also needs updating (i.e., is Astrophysical Journal the correct journal name?)

JostMigenda commented 3 years ago

@arfon Merged your PR, thanks! ApJ is still correct; though I’ve noticed that the official name is "The Astrophysical Journal"—I’ve updated that.

I’ve just tagged a new release (v0.7.3) and archived it on Zenodo (10.5281/zenodo.4688040). Please let me know if you need anything else!

xuanxu commented 3 years ago

@whedon set v0.7.3 as version

whedon commented 3 years ago

OK. v0.7.3 is the version.

xuanxu commented 3 years ago

@whedon set 10.5281/zenodo.4688040 as archive

whedon commented 3 years ago

OK. 10.5281/zenodo.4688040 is the archive.

xuanxu commented 3 years ago

@whedon accept

whedon commented 3 years ago

Attempting dry run of processing paper acceptance...

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/2230

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

@whedon accept deposit=true

whedon commented 3 years ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1140/epjc/s10052-020-7977-8 is OK
- 10.1016/S0370-2693(98)00087-2 is OK
- 10.1016/0370-1573(79)90010-3 is OK
- 10.1016/S0375-9474(99)00152-9 is OK
- 10.1103/PhysRevD.66.013007 is OK
- 10.1093/mnras/sty2164 is OK
- 10.1103/PhysRevD.86.125031 is OK
- 10.1086/375130 is OK
- 10.1088/1475-7516/2006/05/012 is OK
- 10.1088/1475-7516/2003/06/006 is OK
- 10.1103/PhysRevD.78.085012 is OK
- 10.1103/PhysRevD.98.103020 is OK
- 10.1103/PhysRevD.89.093001 is OK
- 10.1016/j.nuclphysb.2016.02.012 is OK
- 10.1103/PhysRevD.76.081301 is OK
- 10.1103/PhysRevLett.103.051105 is OK
- 10.1103/PhysRevLett.118.021101 is OK
- 10.1103/PhysRevLett.106.091101 is OK
- 10.1103/PhysRevLett.104.191102 is OK
- 10.1103/PhysRevD.74.105014 is OK
- 10.1103/PhysRevLett.97.241101 is OK
- 10.1088/1475-7516/2005/04/002 is OK
- 10.1103/PhysRevD.62.033007 is OK
- 10.1103/PhysRevD.68.093013 is OK
- 10.1093/ptep/pty134 is OK
- 10.1103/PhysRevC.98.034613 is OK
- 10.1103/PhysRevD.51.6146 is OK
- 10.1016/0370-2693(71)90050-5 is OK
- 10.1016/j.astropartphys.2016.04.003 is OK
- 10.1103/PhysRevC.67.035502 is OK
- 10.1016/s0370-2693(03)00616-6 is OK
- 10.1088/0067-0049/205/1/2 is OK
- 10.1038/s41592-019-0686-2 is OK
- 10.1109/MCSE.2011.37 is OK

MISSING DOIs

- None

INVALID DOIs

- None

kyleniemeyer commented 3 years ago

OK, this looks good to me!

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

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

Any issues? Notify your editorial technical team...

kyleniemeyer commented 3 years ago

Congrats @JostMigenda on your article's publication in JOSS!

Many thanks to @bradkav and @benjaminrose for reviewing this, and @xuanxu for editing.

openjournals / joss-reviews