search

openjournals / joss-reviews

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

[REVIEW]: FAME-Io: Configuration tools for complex agent-based simulations #4958

Closed editorialbot closed 1 year ago

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@dlr_fn<!--end-author-handle-- (Felix Nitsch) Repository: https://gitlab.com/fame-framework/fame-io Branch with paper.md (empty if default branch): 139-prepare-publication Version: v1.7 Editor: !--editor-->@fraukewiese<!--end-editor-- Reviewers: @imcatta, @robbiemorrison Archive: 10.5281/zenodo.4314337

Status

status

Status badge code:

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

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

@imcatta & @robbiemorrison, 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 @fraukewiese 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 @imcatta

📝 Checklist for @robbiemorrison

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.14 s (514.2 files/s, 47126.8 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          53            875            328           4028
Markdown                         8            180              0            745
TeX                              1             17              0            180
YAML                             8              6              0             56
-------------------------------------------------------------------------------
SUM:                            70           1078            328           5009
-------------------------------------------------------------------------------

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

Wordcount for paper.md is 1032

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

OK DOIs

- 10.1155/2017/7494313 is OK
- 10.1155/2017/1967645 is OK
- 10.1016/j.rser.2015.12.169 is OK
- 10.3390/en13153920 is OK
- 10.1016/j.energy.2016.03.038 is OK
- 10.1007/s10462-009-9105-x is OK
- 10.18564/jasss.4129 is OK
- 10.1016/j.rser.2014.02.003 is OK
- 10.1080/14693062.2020.1824891 is OK
- 10.3390/en13205350 is OK
- 10.1016/j.energy.2018.03.092 is OK
- 10.1016/j.erss.2018.10.021 is OK
- 10.1109/EEM49802.2020.9221924 is OK
- 10.5281/zenodo.5726738 is OK

MISSING DOIs

- None

INVALID DOIs

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

imcatta commented 1 year ago

Review checklist for @imcatta

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

robbiemorrison commented 1 year ago

Review checklist for @robbiemorrison

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

fraukewiese commented 1 year ago

@imcatta @robbiemorrison : How is the review going?

robbiemorrison commented 1 year ago

Hi Frauke, I'll look this weekend. Robbie

On 15/12/2022 15.55, fraukewiese wrote:

@imcatta https://github.com/imcatta @robbiemorrison https://github.com/robbiemorrison : How is the review going?

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4958#issuecomment-1353220068, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF56KEU3UGODSSXHBTFBGGDWNMWNZANCNFSM6AAAAAASIYPPUY. You are receiving this because you were mentioned.Message ID: @.***>

-- Robbie Morrison Address: Schillerstrasse 85, 10627 Berlin, Germany Phone: +49.30.612-87617

robbiemorrison commented 1 year ago

I presume I can simply post my review report to this thread.

Overall

The package in question should fulfill a useful role and would assist users and creators with their data management tasks. It is sensible, design‑wise, to create a clear separation between data preparation functionality and numerical processing and then bridge the two using a stable interface or protocol.

More general comments

In terms of the title, FAME‑Io is more than "Configuration tools". Perhaps better would be "Data management tools for complex agent‑based simulations" or "Workflow management for complex agent‑based simulations".

Some comments in point form:

Figure 1 indicates a GUI but this aspect is not much covered. A screenshot might be warranted, if available and informative? Otherwise that kind of work‑in‑progress should be indicated on figure 1, perhaps even grayed out? FAME‑Gui is described as being "in beta release" at one point.

More specific comments

Some specific comments:

And a couple of suggestions, which are entirely optional:

Language and lead paragraph

I appreciate that the authors are likely non‑native speakers of English. But the text should be improved upon.

To that end, I rewrote the first paragraph to provide better flow. The text below is simply a suggestion and the authors can elect to use all, some, or none of it:

We present FAME‑Io, a python package designed to help the users and creators of agent-based simulation models (ABM) better manage the preparation and processing of their input and output datasets. The package was built with the needs of researchers in mind. FAME‑Io was specifically developed to interface with the FAME‑Core framework but has been applied to the AMIRIS model and is intended to be extensible to other ABM models. The software offers good logging capabilities, shell‑integrated help, and online documentation as well as extensive pre‑run integrity checks and interpreted warning messages. It also allows individual data components to be easily extracted and used in secondary workflows. The code itself is operating system independent and follows good software development practices. Test coverage, at the time of writing, is 93% and the project employs continuous integration and offers frequent releases.

Further, should the authors wish, I am happy to mark up a printout with some copy‑editing suggestions.

▢

robbiemorrison commented 1 year ago

@fraukewiese: Pinging you after two hours regarding the above. What is the process for collecting reviewer comments and embarking on an iteration? Is this agile development or is it necessary to wait until all the reviews are in before looping? Please advise.

imcatta commented 1 year ago

Dear authors, in order to complete my checklist some actions on your side are requested.

  • [x] Contribution and authorship: Has the submitting author (@dlr_fn) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Aurelien Regat-Barrel (arb@cyberkarma.net) has made a number of contributions to the codebase, but he is not listed as an author of the submission. Please clarify his role in the project.

  • [x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.

The installation of the software and its dependencies are handled automatically via pip. However, Python version constraints (i.e. minimum version, ...) should be stated in the README and/or in the project wiki.

  • [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).

There is a folder in the git project containing some examples of FAME-io configuration files, but at first glance it is not immediately clear how to use them. The authors should consider adding a "Quick Start" or "Example usage" section to the README and/or to the project wiki.

  • [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

Information on how to report issues and seek support should be added to the README and/or to the project wiki.

  • [x] State of the field: Do the authors describe how this software compares to other commonly-used packages?

Since this is a configuration tool for a niche framework such as FAME, it is difficult to identify directly comparable packages. In any case, a comparison with other libraries that allow ABM development via code should be reported in the manuscript.

  • [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?

The paper "AMIRIS: Agent-based market model for the investigation of renewable and integrated energy systems" has not been accepted yet on JOSS. This fact should be stated.

Minor improvements

A link from the README should be updated. For the sake of simplicity, I submitted a merge request.

fxnitsch commented 1 year ago

Dear @robbiemorrison, dear @imcatta, on behalf of all authors, I would like to thank you very much for your detailed review. I will return to the office next week and start addressing the issues and suggestions you raised.

robbiemorrison commented 1 year ago

@fxnitsch Noted, R

fxnitsch commented 1 year ago

@editorialbot generate pdf

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:

fxnitsch commented 1 year ago

Dear @robbiemorrison, dear @imcatta,

Once again, thank you for your much appreciated reviews.

We have thoroughly addressed all the points raised and are pleased to provide you with a revised version of our paper and repository.

In detail, we have revised the abstract, provided more detailed instructions for use with the examples provided, added issue templates, acknowledged Aurelien Regat-Barrel who advised us on the interface of FAME-Io with FAME-Gui, expanded the section on the state of the field, and made a general refinement to the text.

Figure 1 is currently being converted to an UML diagram as proposed. We are coordinating the updated version with the authors of the FAME-Core submission to achieve a more streamlined look. This takes a little more time and we will add the updated figure shortly.

A complete list of changes with associated commits can be found in our merge request.

We hope that the improved version addresses all your issues raised.

Please do not hesitate to contact us in case of any open questions.

Best regards, Felix

fxnitsch commented 1 year ago

@editorialbot generate pdf

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:

fxnitsch commented 1 year ago

Figure 1 is currently being converted to an UML diagram as proposed. We are coordinating the updated version with the authors of the FAME-Core submission to achieve a more streamlined look. This takes a little more time and we will add the updated figure shortly.

FYI: Figure 1 has been updated.

imcatta commented 1 year ago

I would like to thank the authors for their careful revision. All my concerns/questions have been adequately addressed.

robbiemorrison commented 1 year ago

Will look at this iteration this weekend .. more soon, R

robbiemorrison commented 1 year ago

Review 2

L00 indicates the PDF line numbering.

One specific concern

My only concern is that I think that the use of YAML files for configuration purposes should be first mentioned in the "Configuration of FAME model inputs section". A sentence like the following at approximately L34:

An overarching configuration is comprised of one or more structured YAML files. Large data structures, like time-series, can be imported from external (CSV) files.

Markup

The paragraph breaks seemed inconsistent and made up of a mixture of single line‑breaks and double line‑breaks?

Small optional tweaks

These tweaks are optional:

Conclusion

From my perspective, this review process is complete. I trust the authors to look at the above matters without the need for a further iteration.

▢

robbiemorrison commented 1 year ago

My review checklist now fully checked off. R

robbiemorrison commented 1 year ago

Just to notify @fraukewiese and @fxnitsch. Please see my final review two posting above. R

fraukewiese commented 1 year ago

Thank you very much @robbiemorrison and @imcatta for your thorough review!

fraukewiese commented 1 year ago

@fxnitsch : Please notify me when you have addressed the minor points in Review2 from robbiemorrison.

fraukewiese commented 1 year ago

@fxnitsch : I would like to wait with the further processing of the submission in case there are points raised in #5087 that actually concern also this submission.

fxnitsch commented 1 year ago

Dear @fraukewiese,

@fxnitsch : Please notify me when you have addressed the minor points in Review2 from robbiemorrison.

We have implemented the latest suggestions by @robbiemorrison.

@fxnitsch : I would like to wait with the further processing of the submission in case there are points raised in #5087 that actually concern also this submission.

Since we also have some cross-references to papers #5087 and #5041, we are happy to wait until the other two submissions have gone through the review process.

Currently we have some placeholders in the references, e.g., "AMIRIS. Submitted to: Journal of Open Source Software", which is being reviewed in #5041. The same is true for #5087. Once the peer review is completed, are the DOIs reserved before the final version of the paper is published? It would be nice to update the cross-references of all three papers with their final DOIs beforehand.

fraukewiese commented 1 year ago

@fxnitsch : Yes that should be possible. I think the DOI of this submission would be 10.21105/joss.04958 . It is 10.21105/joss. followed by the number of the review issue.

fraukewiese commented 1 year ago

@editoralbot generate pdf

fraukewiese commented 1 year ago

@editorialbot generate pdf

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:

fraukewiese commented 1 year ago

@dlr_fn All three related submissions #5087 #4958 #5041 have now been reviewed and we can proceed with the final steps. Please make sure the cross-references between the papers are correct and updated - the final DOIs should be 10.21105/joss.05087 10.21105/joss.04958 10.21105/joss.05041

At this point could you:

I can then move forward with accepting the submission.

robbiemorrison commented 1 year ago

Another option for archiving source code-oriented software is Software Heritage. You will gain a more useful SWHID instead of a DOI. Some background: https://forum.openmod.org/t/3788

danielskatz commented 1 year ago

While Software Heritage is providing a useful service, the JOSS workflow requires a DOI at this point.

robbiemorrison commented 1 year ago

Okay, useful to know. I just checked. fameio was routinely archived on 20 February 2023 as release 1.7:

A bit stale but one can provide active uploads to Software Heritage also.

fxnitsch commented 1 year ago

Dear @fraukewiese

please find our latest tagged release v1.7 here: https://gitlab.com/fame-framework/fame-io/-/tags/v1.7 The archived and checked version v1.7 is published to Zenodo here: https://doi.org/10.5281/zenodo.4314337

Thank you again for the very smooth review process!

fraukewiese commented 1 year ago

@editorialbot set v1.7 as version

editorialbot commented 1 year ago

Done! version is now v1.7

fraukewiese commented 1 year ago

@editorialbot set 10.5281/zenodo.4314337 as archive

editorialbot commented 1 year ago

Done! Archive is now 10.5281/zenodo.4314337

fraukewiese commented 1 year ago

@editorialbot recommend-accept

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

OK DOIs

- 10.1155/2017/7494313 is OK
- 10.1155/2017/1967645 is OK
- 10.1016/j.rser.2015.12.169 is OK
- 10.3390/en13153920 is OK
- 10.1016/j.energy.2016.03.038 is OK
- 10.1007/s10462-009-9105-x is OK
- 10.18564/jasss.4129 is OK
- 10.1016/j.rser.2014.02.003 is OK
- 10.1080/14693062.2020.1824891 is OK
- 10.3390/en13205350 is OK
- 10.1016/j.energy.2018.03.092 is OK
- 10.1016/j.erss.2018.10.021 is OK
- 10.1109/EEM49802.2020.9221924 is OK
- 10.5281/zenodo.5726738 is OK
- 10.1177/0037549712462620 is OK

MISSING DOIs

- None

INVALID DOIs

- 10.21105/joss.05087 is INVALID
- 10.21105/joss.05041 is INVALID
- https://doi.org/10.1016/j.rser.2018.08.002 is INVALID because of 'https://doi.org/' prefix
fraukewiese commented 1 year ago

@fxnitsch : Could you please check the invalid DOI listed third. I think you just have to delete the prefix.

fraukewiese commented 1 year ago

@danielskatz : How can we handle the two invalid DOIs? Those two are just invalid because they are cross-references to two other submissions #5087 and #5041 that are not yet published but at the same processing stage as this one.

fxnitsch commented 1 year ago

@editorialbot check references

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

OK DOIs

- 10.1155/2017/7494313 is OK
- 10.1155/2017/1967645 is OK
- 10.1016/j.rser.2015.12.169 is OK
- 10.3390/en13153920 is OK
- 10.1016/j.energy.2016.03.038 is OK
- 10.1007/s10462-009-9105-x is OK
- 10.18564/jasss.4129 is OK
- 10.1016/j.rser.2014.02.003 is OK
- 10.1080/14693062.2020.1824891 is OK
- 10.3390/en13205350 is OK
- 10.1016/j.energy.2018.03.092 is OK
- 10.1016/j.erss.2018.10.021 is OK
- 10.1109/EEM49802.2020.9221924 is OK
- 10.5281/zenodo.5726738 is OK
- 10.1016/j.rser.2018.08.002 is OK
- 10.1177/0037549712462620 is OK

MISSING DOIs

- None

INVALID DOIs

- 10.21105/joss.05087 is INVALID
- 10.21105/joss.05041 is INVALID
fxnitsch commented 1 year ago

@fxnitsch : Could you please check the invalid DOI listed third. I think you just have to delete the prefix.

Fixed!

danielskatz commented 1 year ago

@danielskatz : How can we handle the two invalid DOIs? Those two are just invalid because they are cross-references to two other submissions #5087 and #5041 that are not yet published but at the same processing stage as this one.

@fraukewiese - I think we can let these go through the process without them yet working, given what you have said, as long as you are sure that both will be accepted.

@arfon - do you agree?

I'm also ccing @openjournals/pe-eics since that's the track this is in...