[REVIEW]: 21cmFAST v3: A Python-integrated C code for generating 3D realizations of the cosmic 21cm signal.

whedon commented 4 years ago

Submitting author: @steven-murray (Steven Murray) Repository: https://github.com/21cmFAST/21cmFAST Version: v3.0.3 Editor: @dfm Reviewer: @sambit-giri, @sultan-hassan Archive: 10.5281/zenodo.4107189

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

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

@sambit-giri & @sultanier, 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 @dfm 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 @sambit-giri

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 (@steven-murray) 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 @sultan-hassan

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 (@steven-murray) 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 4 years ago

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

Can't find any papers to compile :-(

steven-murray commented 4 years ago

@sultanier the updates to the hyperlinks have been merged. The benchmarks have also been added to the paper, but I can't seem to get it to compile. @dfm do you have any idea what could be going wrong there? The paper is definitely still there in the joss-paper branch...

@sambit-giri I'm not sure if all your questions/comments were addressed adequately?

sambit-giri commented 4 years ago

@steven-murray My questions/comments were addressed. I thought that I had checked all the points, which I noticed now that I hadn't. Sorry for the delay.

sambit-giri commented 4 years ago

@dfm I am done with the reviewing process. Is there any other unfinished formality?

dfm commented 4 years ago

@sambit-giri and @sultanier: You're both all set! Thank you for your time and for your constructive reviews!!

@steven-murray: I'll see if I can figure out the whedon issues and get back to you with next steps.

dfm commented 4 years ago

@whedon generate pdf from branch joss-paper

whedon commented 4 years ago

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

whedon commented 4 years ago

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

Can't find any papers to compile :-(

dfm commented 4 years ago

Pinging @openjournals/dev to see if there are any thoughts about why @whedon can't find the paper. I can confirm that whedon.theoj.org is able to find it. Thanks!

arfon commented 4 years ago

@whedon generate pdf from branch joss-paper

whedon commented 4 years ago

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

whedon commented 4 years ago

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

Can't find any papers to compile :-(

arfon commented 4 years ago

Weird, I need to dig into this more. Pls hold.

dfm commented 4 years ago

Thanks @arfon!

sultan-hassan commented 4 years ago

Great, looking forward to see the last version of the paper. congrats!

Sultan

p.s. just a note for your reference, I have changed my username to @sultan-hassan

arfon commented 4 years ago

OK, I've figured out what's going on. I've not quite figured out the 'proper' fix here but I have a short-term solution. tl;dr – delete (or rename) the joss-paper folder in your default (master) branch and everything should be fine.

If you're interested, here's what's going on:

The issue here is that we're asking Whedon to act on the joss-paper branch but there's also a folder named joss-paper in the default (master) branch.

This confuses Git¹, and I thought that this commit would address the issue (it does locally with my local Git version) but Whedon's Git is older and this flag doesn't seem to work there.

Ultimately I need to spend more time debugging this but I'm out of time this evening and don't want to block this further.

¹ https://stackoverflow.com/questions/25322335/git-change-branch-when-file-of-same-name-is-present

steven-murray commented 4 years ago

I'll just merge the PR and we can build off master :-)

steven-murray commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

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

sultan-hassan commented 4 years ago

Hi @dfm, I have changed my github username, is it possible to update that in the paper too? thanks.

dfm commented 4 years ago

@sultan-hassan: yes thanks for the heads up! I'll fix it in the process of the final edit.

steven-murray commented 4 years ago

@whedon generate pdf from branch joss-paper-fixes

whedon commented 4 years ago

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

whedon commented 4 years ago

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

steven-murray commented 4 years ago

@whedon generate pdf from branch joss-paper-fixes

whedon commented 4 years ago

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

whedon commented 4 years ago

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

sambit-giri commented 4 years ago

Congrats to all the authors!

sambit-giri commented 4 years ago

@dfm I noticed that the author's list of the paper has been modified. I have worked with Catherine Watkinson in a project where both of us were primary authors (https://ui.adsabs.harvard.edu/abs/2019MNRAS.482.2653W/abstract). Does this violate the JOSS conflict of interest policy?

dfm commented 3 years ago

@sambit-giri: Thanks for the heads-up! I'll check in with the editorial board and get back to you. Thanks!

dfm commented 3 years ago

@steven-murray: I've opened a PR to the 21cmfast repo with a few minor formatting edits to the paper. Please take a look (and preferably accept) those changes and then go through the following steps:

I see now that there is also a joss-paper-fixes branch that will have one conflict with my PR. Please resolve those issues and confirm which branch the final submission will live on.
Get whedon to compile the paper and then read through the proofs carefully (especially checking things like author names and affiliations, but also general typos that we might have all missed) to make sure that you're happy. It's much harder to change things later! I believe that @sultan-hassan's username should be updated in the new version of the paper, but we should also check that.
Bump the version number and report the version number back here.
Create a tagged release and archive this release somewhere like Zenodo. The metadata of that archive (title and author list) must exactly match the JOSS paper. You can edit the metadata after creating the archive on most services. Report the DOI for the archive in this thread.

Let me know if you have any questions!

dfm commented 3 years ago

@sambit-giri: I've checked with the editorial team and we've agreed that this does not constitute a conflict-of-interest in this case since you had already done most of your review and since it has been noted here. If @steven-murray or any of the co-authors have any concerns, please let me know here or offline, but otherwise we'll proceed. Thanks!

steven-murray 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:

steven-murray commented 3 years ago

OK @dfm, I've merged the paper fixes, and the proof looks good.

As for the version update -- does that need to be a new version? Can we point to the last published version (3.0.2)? If not that's fine, but may be a day or so before we can release a new version.

dfm commented 3 years ago

The version for the paper should represent the current state of the code base and docs. It looks to me like there have been a fair number of updates since 3.0.2 (July 2020) so I think you should make a new one. If you think otherwise, please let me know!

steven-murray commented 3 years ago

You're right, @dfm. Made a new version.

New version is v3.0.3 and DOI is 10.5281/zenodo.4107189.

dfm commented 3 years ago

Thanks! Can you edit the Zenodo meta data so that the title is "21cmFAST v3: A Python-integrated C code for generating 3D realizations of the cosmic 21cm signal"?

steven-murray commented 3 years ago

:facepalm: done.

dfm commented 3 years ago

@whedon set 10.5281/zenodo.4107189 as archive

whedon commented 3 years ago

OK. 10.5281/zenodo.4107189 is the archive.

dfm commented 3 years ago

@whedon set v3.0.3 as version

whedon commented 3 years ago

OK. v3.0.3 is the version.

dfm 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.1086/521806 is OK
- 10.1111/j.1365-2966.2010.17731.x is OK
- 10.1086/423025 is OK
- 10.1046/j.1365-8711.2002.04999.x is OK
- 10.1103/PhysRevLett.123.131301 is OK
- 10.1103/PhysRevD.100.063538 is OK
- 10.1093/mnras/staa1131 is OK
- 10.1103/PhysRevLett.121.121301 is OK
- 10.1093/mnras/stv571 is OK

MISSING DOIs

- 10.1093/mnras/staa2797 may be a valid DOI for title: Reionization inference from the CMB optical depth and E-mode polarization power spectra

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

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

@whedon accept deposit=true

dfm commented 3 years ago

@steven-murray: I see now that I missed that updated reference. It looks like https://ui.adsabs.harvard.edu/abs/2020MNRAS.499..550Q/abstract has now been published so can you update the bibtex entry to the published version?

EiC: I'll remove the recommend-accept label until @steven-murray fixes that. Sorry all about the false alarm!

steven-murray commented 3 years ago

Thanks for spotting that @dfm. It's updated in master now.

dfm commented 3 years ago

@whedon check references

openjournals / joss-reviews