openjournals / joss-reviews

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

[REVIEW]: Black-it: A Ready-to-Use and Easy-to-Extend Calibration Kit for Agent-based Models #4622

Closed editorialbot closed 2 years ago

editorialbot commented 2 years ago

Submitting author: !--author-handle-->@AldoGL<!--end-author-handle-- (Aldo Glielmo) Repository: https://github.com/bancaditalia/black-it/ Branch with paper.md (empty if default branch): joss-paper Version: v0.1.2 Editor: !--editor-->@drvinceknight<!--end-editor-- Reviewers: @Athene-ai, @aseyq Archive: 10.5281/zenodo.7273564

Status

status

Status badge code:

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

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

@Athene-ai & @aseyq, 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 @drvinceknight 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 @Athene-ai

📝 Checklist for @aseyq

editorialbot commented 2 years 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 2 years ago
Software report:

github.com/AlDanial/cloc v 1.88  T=0.14 s (846.0 files/s, 167492.8 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          78           1506           2894           4028
Markdown                        20            290              0            762
YAML                             5             34            121            544
Jupyter Notebook                 7              0          12284            528
TeX                              1             43              6            230
INI                              2             23              2            152
make                             1             51             13            149
Bourne Shell                     4             27             16            104
TOML                             1              5              0             86
Julia                            1             12             11             35
JSON                             1              0              0              1
-------------------------------------------------------------------------------
SUM:                           121           1991          15347           6619
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 2 years ago

Wordcount for paper.md is 1425

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

OK DOIs

- 10.1080/0022250X.1971.9989794 is OK
- 10.1098/rsif.2007.1206 is OK
- 10.1371/journal.pcbi.1009146 is OK
- 10.2139/ssrn.2850414 is OK
- 10.2139/ssrn.3891583 is OK
- 10.1145/264029.264064 is OK
- 10.1145/2739482.2768468 is OK
- 10.1098/rsos.150703 is OK
- 10.48550/arXiv.1605.00998 is OK
- 10.1016/j.jedc.2017.01.014 is OK
- 10.1016/j.jedc.2018.03.011 is OK
- 10.1007/s10614-021-10095-9 is OK
- 10.48550/arXiv.2202.00625 is OK
- 10.1016/j.jedc.2020.103859 is OK
- 10.1016/j.jempfin.2009.06.006 is OK
- 10.1016/j.ecosta.2017.01.006 is OK
- 10.1016/S0165-1889(98)00011-6 is OK
- 10.1038/30918 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 2 years ago

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

aseyq commented 2 years ago

Review checklist for @aseyq

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Athene-ai commented 2 years ago

Review checklist for @Athene-ai

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Athene-ai commented 2 years ago

I have just finished my review and here below are some additional comments.

Additional comments:

Contribution and authorship: Seem that in the contribution description is missing

-Installation_: The reviewer suggests expanding the installation description especially if the package will be installed by a newbie

Example usage: This part should be implemented

Functionality documentation and Automated tests: these parts should be better implemented

aseyq commented 2 years ago

I've also finished my review and here are my comments:

The paper introduced Black-it, a calibration tool which helps finding sets of parameters to fit the empirical data best for ABMs in Python. The core functionality works in three steps: it samples from the potential set of parameters, simulates for the selected parameters, and evaluates the fit with a loss function. In my opinion, the software seems very useful, the paper is well-written and concise.

Here are my suggestions for improvements:

Functionality documentation The documentation and the examples seem adequate to me in general but the code reference seems would really need some benefit from some work. In the current form difficult to understand the functions and differences of components if one is not familiar with the methods.

State of the field I know it is difficult to navigate in the Python package ecosystem but are there other common packages doing a similar job? And if not, maybe stating how it compares to writing your own calibration routines with the use generic optimization tools (like those provided by SciPy) Python for the ABM.

and in addition, I think both the paper and the software would benefit if you could better elaborate how to use Black-it with common ABM frameworks on Python, naming a few of them. And related to that, maybe to state that it is a framework (and potentially language) agnostic tool.

AldoGl commented 2 years ago

Thank you @Athene-ai for taking the time to learn about Black-it and for your comments. We improved our work following your suggestions, the details are in the following.

The list of the Authors of Black-it appears in this page https://github.com/bancaditalia/black-it/blob/main/AUTHORS.md. In this "Authors" page there is a link to the “Contributing” page, this one https://github.com/bancaditalia/black-it/blob/main/CONTRIBUTING.md. This link was broken because of a typo. We fixed it.

We made a substantial effort in making the installation as smooth as possible, particularly through the use of the “Poetry” dependency manager. As a result of our careful work it should be especially simple for users to install Black-it since, by running “pip install black-it”, all dependencies will be seamlessly installed along with the package. This is why the instructions for the installation are so concise and, unless you have a strong opinion on this, we would like to leave the installation description short and simple. This is one of the beauties of the Python package ecosystem after all.

The "Quick Example" provided in the README does not work via a simple copy and paste. This was perhaps not obvious, and have clarified the needed steps in the description.

We further improved the documentation visibility and quality, as also suggested by Reviewer @aseyq (please refer to the other response message below for more details on this). To improve the visibility of the Automated tests deployed we added a “Code coverage” badge to the GitHub landing page, showing that the code coverage is currently 99%.

AldoGl commented 2 years ago

Thank you @aseyq for taking the time to learn about our work and for assessing our package and manuscript.

Following your suggestions we did the following.

aseyq commented 2 years ago

Dear @AldoGl, Thank you very much for the work done. I think this is a very nice piece of software and I am sure the community will benefit from it. I am looking forward to seeing it published.

Just let me write my comments:

Best,

AldoGl commented 2 years ago

Dear @aseyq,

Thank you for your kind assessment of the software, and for appreciating our revision work on the documentation and on the manuscript.

Your comments on the interface between Black-it and ABM simulators are very helpful. To follow up on them, and on other independent feedback we received in that direction, we are willing to facilitate as much as possible the integration of Black-it with models from the ABM community which, as you pointed out, is wide and diverse.

To that aim, the immediate option is to improve the documentation, as we have done already in response to your previous report, and as we are continuing to do (see, e.g. the PR https://github.com/bancaditalia/black-it/pull/12 where we address specifically the integration of Black-it with Java simulators). A second option we are considering in the medium term, is to change the interface via the addition of specific abstractions other than a simple function. This novel interface, however, must be engineered with caution as the cost of making the interface arbitrarily more complex than a simple and intuitive Python function might easily overtake the potential benefits.

Thank you once again for your review work!

AldoGl commented 2 years ago

Dear @drvinceknight,

I am writing to you to enquire about the status of our manuscript, to make sure that the review process is moving forward smoothly, and to know whether we can facilitate the process in any way at this stage.

Best wishes, Aldo

danielskatz commented 2 years ago

👋 @Athene-ai - can you take another look at the responses from the author and later discussion, and see if you can check off more of your review criteria, then restate your concerns that prevent you from checking off any of the others?

Athene-ai commented 2 years ago

@danielskatz I have just updated my checklist

danielskatz commented 2 years ago

@Athene-ai - thanks!

danielskatz commented 2 years ago

@AldoGl - it looks like this is ready to proceed, which I'll help @drvinceknight with.

At this point could you:

I can then move forward with proofreading the work, and then moving it to acceptance.

AldoGl commented 2 years ago

Dear professor @danielskatz,

Thank you for your message, and thank you, @drvinceknight, @aseyq and @Athene-ai for the your editorial/review work.

I made a tagged release of v0.1.2 of the software, you can find it here https://github.com/bancaditalia/black-it/tree/v0.1.2.

I archived the same release on Zenodo with the DOI 10.5281/zenodo.7273564, this is the corresponding link https://doi.org/10.5281/zenodo.7273564.

I checked that the metadata of the Zenodo archive are correct, and I included the ORCID number of the authors (when available).

Please let me know how I can further help the publication process.

danielskatz commented 2 years ago

@editorialbot set v0.1.2 as version

editorialbot commented 2 years ago

Done! version is now v0.1.2

danielskatz commented 2 years ago

@editorialbot set 10.5281/zenodo.7273564 as archive

editorialbot commented 2 years ago

Done! Archive is now 10.5281/zenodo.7273564

danielskatz commented 2 years ago

@editorialbot recommend-accept

editorialbot commented 2 years ago
Attempting dry run of processing paper acceptance...
editorialbot commented 2 years ago

:warning: Error preparing paper acceptance. The generated XML metadata file is invalid.

ID figU003Ascript already defined
IDREFS attribute rid references an unknown ID "figU003Afeatures"
IDREFS attribute rid references an unknown ID "figU003Afeatures"
editorialbot commented 2 years ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1080/0022250X.1971.9989794 is OK
- 10.1098/rsif.2007.1206 is OK
- 10.1371/journal.pcbi.1009146 is OK
- 10.2139/ssrn.2850414 is OK
- 10.2139/ssrn.3891583 is OK
- 10.1145/264029.264064 is OK
- 10.1145/2739482.2768468 is OK
- 10.1098/rsos.150703 is OK
- 10.48550/arXiv.1605.00998 is OK
- 10.1016/j.jedc.2017.01.014 is OK
- 10.1016/j.jedc.2018.03.011 is OK
- 10.1007/s10614-021-10095-9 is OK
- 10.48550/arXiv.2202.00625 is OK
- 10.1016/j.jedc.2020.103859 is OK
- 10.1016/j.jempfin.2009.06.006 is OK
- 10.1016/j.ecosta.2017.01.006 is OK
- 10.1016/S0165-1889(98)00011-6 is OK
- 10.1038/30918 is OK
- 10.1038/s41592-019-0686-2 is OK

MISSING DOIs

- None

INVALID DOIs

- None
danielskatz commented 2 years ago

👋 @xuanxu - can you check on the error above for me? I wonder if this is an issue with having multiple minipages within a figure, each with a label? Otherwise, I don't see a problem in how the labels are created and used, but maybe I'm missing something?

xuanxu commented 2 years ago

The origin of the error is that pandoc (the tool we use to produce pdf and metadata JATS files) can't handle subfigures like the fig1 and 2 in the paper.md. There are two options to solve this: One solution is to combine both panels into a single image. Another solution is to include the jats code into the paper.md file so pandoc can use it. I've opened a pull request that should solve the problem using the second option.

AldoGl commented 2 years ago

Thank you very much @xuanxu! I have merged your pull request.

danielskatz commented 2 years ago

@editorialbot recommend-accept

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

OK DOIs

- 10.1080/0022250X.1971.9989794 is OK
- 10.1098/rsif.2007.1206 is OK
- 10.1371/journal.pcbi.1009146 is OK
- 10.2139/ssrn.2850414 is OK
- 10.2139/ssrn.3891583 is OK
- 10.1145/264029.264064 is OK
- 10.1145/2739482.2768468 is OK
- 10.1098/rsos.150703 is OK
- 10.48550/arXiv.1605.00998 is OK
- 10.1016/j.jedc.2017.01.014 is OK
- 10.1016/j.jedc.2018.03.011 is OK
- 10.1007/s10614-021-10095-9 is OK
- 10.48550/arXiv.2202.00625 is OK
- 10.1016/j.jedc.2020.103859 is OK
- 10.1016/j.jempfin.2009.06.006 is OK
- 10.1016/j.ecosta.2017.01.006 is OK
- 10.1016/S0165-1889(98)00011-6 is OK
- 10.1038/30918 is OK
- 10.1038/s41592-019-0686-2 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 2 years ago

:wave: @openjournals/csism-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/3673, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

danielskatz commented 2 years ago

👋 @AldoGl - I have some suggested small changes in https://github.com/bancaditalia/black-it/pull/22 - please merge this, or let me know what you disagree with, then we can proceed to acceptance

AldoGl commented 2 years ago

Thank you @danielskatz for your work, I just merged the PR.

danielskatz commented 2 years ago

@editorialbot recommend-accept

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

OK DOIs

- 10.1080/0022250X.1971.9989794 is OK
- 10.1098/rsif.2007.1206 is OK
- 10.1371/journal.pcbi.1009146 is OK
- 10.2139/ssrn.2850414 is OK
- 10.2139/ssrn.3891583 is OK
- 10.1145/264029.264064 is OK
- 10.1145/2739482.2768468 is OK
- 10.1098/rsos.150703 is OK
- 10.48550/arXiv.1605.00998 is OK
- 10.1016/j.jedc.2017.01.014 is OK
- 10.1016/j.jedc.2018.03.011 is OK
- 10.1007/s10614-021-10095-9 is OK
- 10.48550/arXiv.2202.00625 is OK
- 10.1016/j.jedc.2020.103859 is OK
- 10.1016/j.jempfin.2009.06.006 is OK
- 10.1016/j.ecosta.2017.01.006 is OK
- 10.1016/S0165-1889(98)00011-6 is OK
- 10.1038/30918 is OK
- 10.1038/s41592-019-0686-2 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 2 years ago

:wave: @openjournals/csism-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/3674, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

danielskatz commented 2 years ago

@editorialbot accept

editorialbot commented 2 years ago
Doing it live! Attempting automated processing of paper acceptance...
editorialbot commented 2 years ago

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

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

Congratulations to @AldoGl (Aldo Glielmo) and co-authors!!

And thanks to @Athene-ai and @aseyq for reviewing, and to @drvinceknight for editing! We couldn't do this without you

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

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

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

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:

AldoGl commented 2 years ago

On behalf of all authors, thank you @danielskatz, @drvinceknight, @Athene-ai, @aseyq and @xuanxu for the fantastic work!