[REVIEW]: pyhf: pure-Python implementation of HistFactory statistical models

[whedon]

Submitting author: @matthewfeickert (Matthew Feickert)
Repository: https://github.com/scikit-hep/pyhf
Version: v0.5.4
Editor: @eloisabentivegna
Reviewer: @suchitakulkarni, @bradkav
Archive: 10.5281/zenodo.4318533

⚠️ JOSS reduced service mode ⚠️

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

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

@suchitakulkarni & @bradkav, 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 @suchitakulkarni

Conflict of interest

• 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

• I confirm that I read and will adhere to the JOSS code of conduct.

General checks

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

Functionality

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

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

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

Conflict of interest

• 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

• I confirm that I read and will adhere to the JOSS code of conduct.

General checks

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

Functionality

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

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

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

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @suchitakulkarni, @bradkav it looks like you're currently assigned to review this paper 🎉.

⚠️ JOSS reduced service mode ⚠️

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.

⭐ Important ⭐

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:

2. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/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]

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

Can't find any papers to compile :-(

[eloisabentivegna]

@whedon generate pdf from branch docs/add-JOSS-paper

[whedon]

Attempting PDF compilation from custom branch docs/add-JOSS-paper. Reticulating splines etc...

[whedon]

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

[bradkav]

Hi @eloisabentivegna, it looks like the invitation at this URL - https://github.com/openjournals/joss-reviews/invitations - has expired (sorry I was a bit slow getting around to it). Could the invitation be sent again? Thanks.

[eloisabentivegna]

@whedon re-invite @bradkav as reviewer

[whedon]

OK, the reviewer has been re-invited.

@bradkav please accept the invite by clicking this link: https://github.com/openjournals/joss-reviews/invitations

[eloisabentivegna]

@suchitakulkarni, @bradkav, were you able to start the review process, e.g. downloading pyhf? Could you post an update?

[matthewfeickert]

@eloisabentivegna @suchitakulkarni @bradkav, in the time since we first submitted pyhf to JOSS pre-review we've also released patch release v0.5.3 and are about to release v0.5.4. Do you want us to rebase the branch that has the JOSS paper in with these number updated, or would you prefer everything remain strictly frozen at v0.5.2?

[eloisabentivegna]

Dear @matthewfeickert, given we are in the early stages of the review process, I think a rebase would not be too disruptive. I leave it up to the authors: if you prefer to publish the latest version with JOSS, please make sure the paper branch is up to date. Once the reviews are further under way, it may be best to keep the code frozen.

[matthewfeickert]

given we are in the early stages of the review process, I think a rebase would not be too disruptive. I leave it up to the authors: if you prefer to publish the latest version with JOSS, please make sure the paper branch is up to date. Once the reviews are further under way, it may be best to keep the code frozen.

Sounds good! I just rebased so I'll trigger wheadon now.

[matthewfeickert]

@whedon generate pdf from branch docs/add-JOSS-paper

[whedon]

Attempting PDF compilation from custom branch docs/add-JOSS-paper. Reticulating splines etc...

[whedon]

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

[matthewfeickert]

Rebased to get v0.5.4 (detailed here) which is the same as v0.5.3 but with some safeguarding of optional dependencies changes.

[matthewfeickert]

@whedon generate pdf from branch docs/add-JOSS-paper

[whedon]

Attempting PDF compilation from custom branch docs/add-JOSS-paper. Reticulating splines etc...

[whedon]

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

[bradkav]

@eloisabentivegna Sorry for the delay. I've now started the review (downloaded & installed pyhf, running some tests etc.). I should be able to provide some comments by the end of the week.

[bradkav]

Review:

pyhf is an incredibly useful tool, which is already an important part of the HEP analysis ecosystem. Installation is straightforward and there are plenty of examples. Before I can recommend publication in JOSS, I just have a few minor comments and suggestions, aimed mostly at making the code more accessible to newcomers.

• Documentation: Some brief examples/documentation are provided in the README. However, it appears that the most complete documentation is at https://scikit-hep.org/pyhf/, so it would be useful to add a link to this more prominently in the README. This would also lead new users more quickly to the many examples at https://scikit-hep.org/pyhf/examples.html.

• Examples: The command-line example in the README - cat << EOF | tee likelihood.json | pyhf cls - doesn't seem to work. Is there a missing likelihood.json file?

• Tests: The examples in the README give the correct output. However, it appears that there are also automated tests in the repo. Is there a straightforward way for the user to run these test and verify that everyone is working fine?

• Paper: The citation syntax specifies that citations should be surrounded with square brackets and separated by semi-colons. Please could you update the paper?

• Paper: Typo in the paper - under "Future Work", " aims to provide support limit setting" should presumably be "aims to provide support for limit setting".

[kratsg]

Hi @bradkav - thanks for reviewing. For the example in the README, you should use the full block of code to the last EOF, that is (see below). Admittedly, this is probably not so obvious for users not familiar with all the features on the command line, so if you have suggestions on how to rephrase this, let us know!

For "Tests" - the straightforward way is through pytest - and just running pytest without any options should run all tests, if the user has checked out the development install of pyhf. How would you like us to document the "straightforward" way or is it enough to mention that we use pytest for all of our tests?

For the rest, we'll work on implementing the changes/fixes. Thanks again.

---

cat << EOF  | tee likelihood.json | pyhf cls
{
    "channels": [
        { "name": "singlechannel",
          "samples": [
            { "name": "signal",
              "data": [12.0, 11.0],
              "modifiers": [ { "name": "mu", "type": "normfactor", "data": null} ]
            },
            { "name": "background",
              "data": [50.0, 52.0],
              "modifiers": [ {"name": "uncorr_bkguncrt", "type": "shapesys", "data": [3.0, 7.0]} ]
            }
          ]
        }
    ],
    "observations": [
        { "name": "singlechannel", "data": [51.0, 48.0] }
    ],
    "measurements": [
        { "name": "Measurement", "config": {"poi": "mu", "parameters": []} }
    ],
    "version": "1.0.0"
}
EOF

as in this screenshot:

[matthewfeickert]

Thanks for updating the archive, @arfon. 👍 This all looks good to me from the pyhf team side now. 🙂

[kyleniemeyer]

Hello @matthewfeickert, I'm the EIC on duty this week, and doing some final checks before publishing.

I'm not sure how we missed this before, but your article is missing the required Statement of Need section, which illustrates the research purpose of the software. You can likely repurpose text already present in the article and just add this section header.

Please let me know when you've made that change; there is no need to redo the Zenodo archive, since that is focused on the software (JOSS itself archives the paper). Thanks!

[matthewfeickert]

I'm not sure how we missed this before, but your article is missing the required Statement of Need section, which illustrates the research purpose of the software. You can likely repurpose text already present in the article and just add this section header.

Ah that's unfortunate. It seems this is an area that should be clarified in the future as an editor and two reviewers all checked it off as being completed in the checklist, so it seems it is unclear to the review team what is required here. It is also not being enforced consistently throughout JOSS as in the last week there have been two JOSS publications missing it:

• 
• 

I'll work on getting this done in the next 24 hours.

Please let me know when you've made that change; there is no need to redo the Zenodo archive, since that is focused on the software (JOSS itself archives the paper). Thanks!

@kyleniemeyer Can you please clarify more on this, as it seems that this has been a point of confusion throughout the review for everyone involved. If JOSS is responsible for archiving the paper and doesn't care if there is a dedicated Zenodo archive for the paper by the software dev team themselves, then why is there the requirement that

The archive title should also match the submission title.

? If the purpose of the Zenodo archive linked in the paper is to be an archive of the software release reviewed and not to capture the paper, then the Zenodo archive for this paper should be the pyhf v0.5.4 release: 10.5281/zenodo.4318533 Though if this requirement about the archive title and the submission paper title matching is a hard one then we'll have to do something else as we don't want to disrupt the archive naming convention we've been using.

[kyleniemeyer]

@matthewfeickert well, this is explicitly listed in the (short) set of items of what a paper should contain. We did change this from a suggestion to a more-formal requirement within the last year, so some papers submitted longer ago will not have it. The fact that some papers have slipped through is just due to us not being perfect. whedon is supposed to check for this upon submission, but currently that check fails if there are compilation issues at that stage—whedon is always getting better, but (like us) is not perfect and misses things in some edge cases.

(I'd also like to point out that this is a fairly small ask, since as I mentioned you can likely just move some of your existing text to be under this header.)

We are going to make whedon check for this more consistently, and also add the explicit requirement to the reviewer checklist to match our docs.

[kyleniemeyer]

@matthewfeickert Regarding the version/archive question, we can certainly revisit that hard naming requirement for the archive. Ultimately our main goal is for the archive (and paper) to reflect the software reviewed.

Reviewing the history of this submission, I do see the confusion about versions... but, I don't think anyone mentioned anything about the paper in the archive, just that we need to link to the version reviewed. The paper itself is not really part of the software, so changes made to that do not need to be captured in the archive. (I think things could get very circular if the paper cites the software with archived version.)

Actually, some authors even keep the paper itself in a separate branch that is never merged with the main branch, and so the software archive never contains the paper in that case.

[matthewfeickert]

explicitly listed in the (short) set of items of what a paper should contain.

Yes, and I know that myself, and I assume all of the reviewers in both our pre-review and review, assumed that this paper had met that given that in all circumstances the item

A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?

was checked off on the reviewers' checklist.

As you mention the docs say

A clear Statement of Need that illustrates the research purpose of the software

but the only place to mention this is meant to be an explicit section, and not just an idea ("is it clear from reading this why someone would need this software?") is in the following

As this short list shows, JOSS papers are only expected to contain a limited set of metadata (see example below), a Statement of Need, Summary, Acknowledgements, and References sections

the example paper and bibliography doesn't make this clear either as it contains sections that are explanatory and not intended to actually appear without differentiating. Also the paper linked in the docs as

You can look at an example accepted paper.

has no Statement of Need in it anywhere.

also add the explicit requirement to the reviewer checklist to match our docs.

I think it would be quite helpful for future submissions if in the docs this could be made explicitly clear that "Statement of Need" is a required section of the paper. It would help if a different example paper that included one was used as well.

so some papers submitted longer ago will not have it.

This would explain why the papers we read for published examples didn't have it — thanks for clarifying.

Regarding the version/archive question, we can certainly revisit that hard naming requirement for the archive. Ultimately our main goal is for the archive (and paper) to reflect the software reviewed.

Great — very happy to hear it. 👍 In light of that goal, @kyleniemeyer can I request that you let us keep our Zenodo naming convention and have whedon change the archive to be the one for the v0.5.4 pyhf release: 10.5281/zenodo.4318533? As that's the release that was reviewed.

I'd also like to point out that this is a fairly small ask, since as I mentioned you can likely just move some of your existing text to be under this header.

This is a very reasonable thing to ask for as an editor, and we're in no way rejecting the idea — rather we're happy for the clarification. We don't want to spend more time making continued revisions though until it is clear what the requirements for publication are and what Zenodo archive version we can use for the paper.

The Statement of Need bit is clear now so I'll tag both you and @eloisabentivegna for review of the draft of it once I have time to revise.

[kyleniemeyer]

I think it would be quite helpful for future submissions if in the docs this could be made explicitly clear that "Statement of Need" is a required section of the paper. It would help if a different example paper that included one was used as well.

Indeed, I've already submitted a PR to correct this, and @danielskatz suggested new/improved whedon commands to check for this in openjournals/joss#861

Great — very happy to hear it. 👍 In light of that goal, @kyleniemeyer can I request that you let us keep our Zenodo naming convention and have whedon change the archive to be the one for the v0.5.4 pyhf release: 10.5281/zenodo.4318533? As that's the release that was reviewed.

Yes, this is fine. I think we all are in favor of supporting projects that have a solid versioning and archiving process in place.

[kyleniemeyer]

@whedon set 10.5281/zenodo.4318533 as archive

[whedon]

OK. 10.5281/zenodo.4318533 is the archive.

[bradkav]

I think it would be quite helpful for future submissions if in the docs this could be made explicitly clear that "Statement of Need" is a required section of the paper. It would help if a different example paper that included one was used as well.

I'd like to briefly jump in (as one of the reviewers) and confirm that it was not at all clear to me that the "Statement of Need" should be an explicit section (rather than just an idea to be covered in the paper). The paper did clearly explain the research purpose of the software, so I was happy to accept it.

In any case, I'm glad to see that @kyleniemeyer has submitted a PR to clarify this.

[suchitakulkarni]

I second @bradkav here and I am glad that this has been taken up to be clarified in further reviews.

[matthewfeickert]

@whedon generate pdf from branch master

[whedon]

Attempting PDF compilation from custom branch master. Reticulating splines etc...

[whedon]

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

[matthewfeickert]

@eloisabentivegna @kyleniemeyer Can you please review the latest draft whedon has built?

Thanks also @kyleniemeyer for openjournals/joss#862 — that's great to see! 👍

[kyleniemeyer]

Thanks @matthewfeickert, that looks good. I'm ready to publish the article at this point.

[kyleniemeyer]

@whedon accept deposit=true

[whedon]

Doing it live! Attempting automated processing of paper acceptance...

[eloisabentivegna]

@eloisabentivegna @kyleniemeyer Can you please review the latest draft whedon has built?

I've reviewed the latest draft and I am happy with it too.

[whedon]

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

[whedon]

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

Here's what you must now do:

0. Check final PDF and Crossref metadata that was deposited 👉 Creating pull request for 10.21105.joss.02823 joss-papers#2075
1. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.02823
2. If everything looks good, then close this review issue.
3. Party like you just published a paper! 🎉🌈🦄💃👻🤘

Any issues? Notify your editorial technical team...

[kyleniemeyer]

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

Many thanks to @suchitakulkarni and @bradkav for reviewing this submission, and @eloisabentivegna for editing it.

(The DOI doesn't seem to be resolving yet, but the paper appears correctly at https://joss.theoj.org/papers/10.21105/joss.02823)

[whedon]

🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉

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.02823/status.svg)](https://doi.org/10.21105/joss.02823)

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

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

This is how it will look in your documentation:

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:

• Volunteering to review for us sometime in the future. You can add your name to the reviewer list here: https://joss.theoj.org/reviewer-signup.html
• Making a small donation to support our running costs here: https://numfocus.org/donate-to-joss

[matthewfeickert]

🚀

Many thanks to @suchitakulkarni and @bradkav for reviewing this submission, and @eloisabentivegna for editing it.

Many thanks to everyone indeed, and to you as well @kyleniemeyer! The pyhf team is deeply appreciative of your time, effort, and support of open source software. Thank you!

(The DOI doesn't seem to be resolving yet, but the paper appears correctly at https://joss.theoj.org/papers/10.21105/joss.02823)

Sounds good. 👍