openjournals / joss-reviews

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

[REVIEW]: BlackBIRDS: Black-Box Inference foR Differentiable Simulators #5776

Closed editorialbot closed 11 months ago

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@arnauqb<!--end-author-handle-- (Arnau Quera-Bofarull) Repository: https://github.com/arnauqb/blackbirds Branch with paper.md (empty if default branch): Version: 1.2 Editor: !--editor-->@rkurchin<!--end-editor-- Reviewers: @rajeshrinet, @marvinschmitt Archive: 10.5281/zenodo.8377044

Status

status

Status badge code:

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

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

@rajeshrinet & @marvinschmitt, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @rkurchin 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 ✨

Checklists

πŸ“ Checklist for @rajeshrinet

πŸ“ Checklist for @marvinschmitt

editorialbot commented 1 year ago

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

For a list of things I can do to help you, just type:

@editorialbot commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@editorialbot generate pdf
editorialbot commented 1 year ago
Software report:

github.com/AlDanial/cloc v 1.88  T=0.06 s (1018.1 files/s, 120804.0 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          36            417            508           1986
SVG                              2              0              0            996
Jupyter Notebook                 7              0           2309            447
Markdown                        12             81              0            278
YAML                             4             22              9            185
TeX                              1             17              0            160
CSS                              1             39             20            104
JavaScript                       1              1              0             15
-------------------------------------------------------------------------------
SUM:                            64            577           2846           4171
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 1 year ago

Wordcount for paper.md is 1229

editorialbot commented 1 year ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.5281/zenodo.4296287 is OK
- 10.21105/joss.05361 is OK
- 10.21105/joss.02505 is OK
- 10.21105/joss.04622 is OK
- 10.21105/joss.05428 is OK
- 10.18637/jss.v100.i07 is OK
- 10.21105/joss.04304 is OK

MISSING DOIs

- 10.1016/j.jempfin.2009.06.006 may be a valid DOI for title: Applying the method of simulated moments to estimate a small agent-based asset pricing model
- 10.2307/3318418 may be a valid DOI for title: Exponential convergence of Langevin distributions and their discrete approximations
- 10.1111/rssb.12158 may be a valid DOI for title: A general framework for updating belief distributions

INVALID DOIs

- None
rkurchin commented 1 year ago

(note to self not to pester @marvinschmitt about review progress before early September πŸ˜‰)

editorialbot commented 1 year ago

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

rajeshrinet commented 1 year ago

Review checklist for @rajeshrinet

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

rkurchin commented 1 year ago

Hi @rajeshrinet and @marvinschmitt, just checking in on review progress here! As you proceed, feel free to open issues/PR's in the project repo; if you do so, link back to this review issue for easy tracking.

rkurchin commented 1 year ago

Aaaand I blatantly disregarded my note to self above, apologies Marvin, feel free to disregard for a few days

marvinschmitt commented 1 year ago

Review checklist for @marvinschmitt

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

marvinschmitt commented 1 year ago

A few comments on the paper itself:

@misc{radev2023bayesflow,
  title = {BayesFlow: Amortized Bayesian Workflows With Neural Networks},
  author = {Stefan T Radev and Marvin Schmitt and Lukas Schumacher and Lasse Elsem\"{u}ller and Valentin Pratz and Yannik Sch\"{a}lte and Ullrich K\"{o}the and Paul-Christian B\"{u}rkner},
  year = {2023},
  publisher= {arXiv},
  url={https://arxiv.org/abs/2306.16015}
}
arnauqb commented 11 months ago

Thank you for the reviewers @marvinschmitt @rajeshrinet for agreeing to review blackbirds. I just came back from annual leave and will go through the comments over the next day(s).

arnauqb commented 11 months ago

A few comments on the paper itself:

  • The in-text citations within parentheses are incorrectly formatted. It's currently [...] simulation models (see e.g. Bayedin et al. (2020)) and should be [...] simulation models (see e.g., Bayedin et al., 2020).
  • A tiny pet peeve of mine: e.g. should be followed by a comma. Just mentioning it because it occurred in the citation example from above.
  • Please add curly braces around {B}ayesian in the .bib file for proper capitalization.
  • The sentence [Our package is used] in forthcoming publications yet to be announced publicly doesn't age well. I would suggest removing it altogether.
  • You could consider adding the BayesFlow package for amortized Bayesian workflows (focus on simulation-based inference) to the list of related software. I'm one of the authors of that package, so this is just a suggestion and strictly optional.
@misc{radev2023bayesflow,
  title = {BayesFlow: Amortized Bayesian Workflows With Neural Networks},
  author = {Stefan T Radev and Marvin Schmitt and Lukas Schumacher and Lasse Elsem\"{u}ller and Valentin Pratz and Yannik Sch\"{a}lte and Ullrich K\"{o}the and Paul-Christian B\"{u}rkner},
  year = {2023},
  publisher= {arXiv},
  url={https://arxiv.org/abs/2306.16015}
}

Thank you for the comments / suggestions, I have now incorporated them into the paper. Latest version: https://github.com/arnauqb/blackbirds/suites/16358519183/artifacts/934384480

rajeshrinet commented 11 months ago

@rkurchin I have completed my first review of the code and the paper. The software looks very good. The paper is well written with a clear statement of need and state of the field.

I found a few problems with installation, but @arnauqb addressed those.

I have made a few suggestions via issues on the code repository, which the authors have handled.

Some comments (optional) for the authors to consider:

marvinschmitt commented 11 months ago

Thank you for addressing my comments @arnauqb. Based on the edits, I have updated my review checklist.

BTW: The BayesFlow paper has just been published in JOSS last week, so you could directly update the reference.

@article{Radev_BayesFlow_Amortized_Bayesian_2023,
    author = {Radev, Stefan T. and Schmitt, Marvin and Schumacher, Lukas and ElsemΓΌller, Lasse and Pratz, Valentin and SchΓ€lte, Yannik and KΓΆthe, Ullrich and BΓΌrkner, Paul-Christian},
    doi = {10.21105/joss.05702},
    journal = {Journal of Open Source Software},
    month = sep,
    number = {89},
    pages = {5702},
    title = {{BayesFlow: Amortized Bayesian Workflows With Neural Networks}},
    url = {https://joss.theoj.org/papers/10.21105/joss.05702},
    volume = {8},
    year = {2023}
}
arnauqb commented 11 months ago

I've updated the BayesFlow reference and added an additional README to the examples folder. Thank you very much for your helpful comments and additions.

I believe this concludes the review @rkurchin ?

rkurchin commented 11 months ago

@editorialbot generate post-review checklist

editorialbot commented 11 months ago

I'm sorry human, I don't understand that. You can see what commands I support by typing:

@editorialbot commands

rkurchin commented 11 months ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

rkurchin commented 11 months ago

@editorialbot check references

editorialbot commented 11 months ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.5281/zenodo.4296287 is OK
- 10.21105/joss.05361 is OK
- 10.1016/j.jempfin.2009.06.006 is OK
- 10.2307/3318418 is OK
- 10.1111/rssb.12158 is OK
- 10.21105/joss.02505 is OK
- 10.21105/joss.04622 is OK
- 10.21105/joss.05428 is OK
- 10.18637/jss.v100.i07 is OK
- 10.21105/joss.04304 is OK
- 10.21105/joss.05702 is OK

MISSING DOIs

- None

INVALID DOIs

- None
rkurchin commented 11 months ago

Thanks everyone! Authors, I'll do an editorial pass over the manuscript and send any comments shortly. In the meantime, the immediate next steps for you are (take a look at the checklist above, too):

  1. Merge any and all changes from this review into your main branch and issue a new version tag. (If you want to merge in the paper, you may, but it is not required that the actual manuscript live into the repo in perpetuity since JOSS will host it and you can simply add a badge link or whatever you like. But the actual changes to software and docs do need to be merged!)
  2. Create a DOI for the contents of the repo at the same commit corresponding to that version tag, e.g. using figshare or Zenodo. Please make sure that the metadata (version number, title, author list, etc.) match those of your manuscript.
  3. Post a comment here with the version number and DOI.
rkurchin commented 11 months ago

@editorialbot generate pdf

editorialbot commented 11 months ago

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

arnauqb commented 11 months ago
rkurchin commented 11 months ago

Editorial comments:

I'm pretty sure this is the fewest editorial comments I've ever given! πŸ‘

Conceptual question: Is it currently the case that everything needs to be in PyTorch? For example, could a model be in JAX, or even in some other AD-able language like Julia? It would be helpful to clarify this, and if it's not currently possible, it's obviously beyond the scope of this review but I would selfishly be thrilled if this could be possible 😁

rkurchin commented 11 months ago

@editorialbot set 1.2 as version

editorialbot commented 11 months ago

Done! version is now 1.2

rkurchin commented 11 months ago

@editorialbot set 10.5281/zenodo.8377044 as archive

editorialbot commented 11 months ago

Done! archive is now 10.5281/zenodo.8377044

arnauqb commented 11 months ago

Editorial comments:

  • line 19: "means to" somehow sounds weird to me and it feels like it should be "means of" although I can't explain why (this should be considered an optional change)
  • second page: be consistent about whether equations in the middle of a sentence have commas after them (I lean towards including them, i.e. adding them in equations (2) and (3) to be consistent with (1))

I'm pretty sure this is the fewest editorial comments I've ever given! πŸ‘

Conceptual question: Is it currently the case that everything needs to be in PyTorch? For example, could a model be in JAX, or even in some other AD-able language like Julia? It would be helpful to clarify this, and if it's not currently possible, it's obviously beyond the scope of this review but I would selfishly be thrilled if this could be possible 😁

Thanks for the comments! Just corrected the small typos and pushed the later paper version to the main branch.

For now, the package only supports PyTorch, so all models need to be in Torch. This is because we require the whole pipeline to use torch.autograd. It would certainly be cool if we could support other frameworks / languages, maybe in the future!

rkurchin commented 11 months ago

@editorialbot recommend-accept

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

OK DOIs

- 10.5281/zenodo.4296287 is OK
- 10.21105/joss.05361 is OK
- 10.1016/j.jempfin.2009.06.006 is OK
- 10.2307/3318418 is OK
- 10.1111/rssb.12158 is OK
- 10.21105/joss.02505 is OK
- 10.21105/joss.04622 is OK
- 10.21105/joss.05428 is OK
- 10.18637/jss.v100.i07 is OK
- 10.21105/joss.04304 is OK
- 10.21105/joss.05702 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 11 months ago

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

Check final proof :point_right::page_facing_up: Download article

If the paper PDF and the deposit XML files look good in https://github.com/openjournals/joss-papers/pull/4607, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

arfon commented 11 months ago

@editorialbot accept

editorialbot commented 11 months ago
Doing it live! Attempting automated processing of paper acceptance...
editorialbot commented 11 months ago

Ensure proper citation by uploading a plain text CITATION.cff file to the default branch of your repository.

If using GitHub, a Cite this repository menu will appear in the About section, containing both APA and BibTeX formats. When exported to Zotero using a browser plugin, Zotero will automatically create an entry using the information contained in the .cff file.

You can copy the contents for your CITATION.cff file here:

CITATION.cff

``` cff-version: "1.2.0" authors: - family-names: Quera-Bofarull given-names: Arnau orcid: "https://orcid.org/0000-0001-5055-9863" - family-names: Dyer given-names: Joel orcid: "https://orcid.org/0000-0002-8304-8450" - family-names: Calinescu given-names: Anisoara orcid: "https://orcid.org/0000-0003-2082-734X" - family-names: Farmer given-names: J. Doyne orcid: "https://orcid.org/0000-0001-7871-073X" - family-names: Wooldridge given-names: Michael orcid: "https://orcid.org/0000-0002-9329-8410" contact: - family-names: Quera-Bofarull given-names: Arnau orcid: "https://orcid.org/0000-0001-5055-9863" - family-names: Dyer given-names: Joel orcid: "https://orcid.org/0000-0002-8304-8450" doi: 10.5281/zenodo.8377044 message: If you use this software, please cite our article in the Journal of Open Source Software. preferred-citation: authors: - family-names: Quera-Bofarull given-names: Arnau orcid: "https://orcid.org/0000-0001-5055-9863" - family-names: Dyer given-names: Joel orcid: "https://orcid.org/0000-0002-8304-8450" - family-names: Calinescu given-names: Anisoara orcid: "https://orcid.org/0000-0003-2082-734X" - family-names: Farmer given-names: J. Doyne orcid: "https://orcid.org/0000-0001-7871-073X" - family-names: Wooldridge given-names: Michael orcid: "https://orcid.org/0000-0002-9329-8410" date-published: 2023-09-28 doi: 10.21105/joss.05776 issn: 2475-9066 issue: 89 journal: Journal of Open Source Software publisher: name: Open Journals start: 5776 title: "BlackBIRDS: Black-Box Inference foR Differentiable Simulators" type: article url: "https://joss.theoj.org/papers/10.21105/joss.05776" volume: 8 title: "BlackBIRDS: Black-Box Inference foR Differentiable Simulators" ```

If the repository is not hosted on GitHub, a .cff file can still be uploaded to set your preferred citation. Users will be able to manually copy and paste the citation.

Find more information on .cff files here and here.

editorialbot commented 11 months ago

🐘🐘🐘 πŸ‘‰ Toot for this paper πŸ‘ˆ 🐘🐘🐘

editorialbot commented 11 months 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/4627
  2. Wait a couple of minutes, then verify that the paper DOI resolves https://doi.org/10.21105/joss.05776
  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...

arfon commented 11 months ago

@rajeshrinet, @marvinschmitt – many thanks for your reviews here and to @rkurchin for editing this submission! JOSS relies upon the volunteer effort of people like you and we simply wouldn't be able to do this without you ✨

@arnauqb – your paper is now accepted and published in JOSS :zap::rocket::boom:

editorialbot commented 11 months 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.05776/status.svg)](https://doi.org/10.21105/joss.05776)

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

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

This is how it will look in your documentation:

DOI

We need your help!

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