search

openjournals / joss-reviews

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

[REVIEW]: Pherosensor-toolbox: a Python package for Biology-Informed Data Assimilation #6863

Closed editorialbot closed 2 months ago

editorialbot commented 5 months ago

Submitting author: !--author-handle-->@slabarthe<!--end-author-handle-- (Simon Labarthe) Repository: https://forgemia.inra.fr/pherosensor/pherosensor-toolbox Branch with paper.md (empty if default branch): Version: v0.1.2 Editor: !--editor-->@Nikoleta-v3<!--end-editor-- Reviewers: @Shreyas911, @mrazomej Archive: 10.5281/zenodo.13834137

Status

status

Status badge code:

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

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

@Shreyas911 & @mrazomej, 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 @Nikoleta-v3 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 @mrazomej

📝 Checklist for @Shreyas911

editorialbot commented 5 months 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 5 months ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1126/science.1183899 is OK
- 10.5281/ZENODO.11506617 is OK
- 10.1142/9789812701831_0006 is OK
- 10.1017/S0021859605005708 is OK
- 10.21105/joss.05150 is OK
- 10.1016/j.envsoft.2014.02.008 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 5 months ago

Software report:

github.com/AlDanial/cloc v 1.90  T=0.09 s (1478.6 files/s, 121929.5 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          67           1512           2132           5221
Cucumber                        32             32              0            440
reStructuredText                16            165            156            219
YAML                             3             16             17            137
TeX                              1              5              0             89
INI                              2              8              1             87
Markdown                         1             19              0             59
DOS Batch                        1              8              1             26
CSV                              1              0              0              9
TOML                             1              2              0              9
make                             1              4              7              9
-------------------------------------------------------------------------------
SUM:                           126           1771           2314           6305
-------------------------------------------------------------------------------

Commit count by author:

    81  MALOU THIBAULT
    67  Simon Labarthe
     1  Thibault Malou
editorialbot commented 5 months ago

Paper file info:

📄 Wordcount for paper.md is 809

✅ The paper includes a Statement of need section

editorialbot commented 5 months ago

License info:

🟡 License found: Other (Check here for OSI approval)

editorialbot commented 5 months ago

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

Nikoleta-v3 commented 5 months ago

Hey @Shreyas911, @mrazomej (@slabarthe) this is the review thread for the paper. All of our communications will happen here from now on.

As a reviewer, the first step is to create a checklist for your review by entering

@editorialbot generate my checklist

as the top of a new comment in this thread.

These checklists contain the JOSS requirements ✅ As you go over the submission, please check any items that you feel have been satisfied. The first comment in this thread also contains links to the JOSS reviewer guidelines.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention https://github.com/openjournals/joss-reviews/issues/6863 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for reviews to be completed within about 2-4 weeks. Please let me know if any of you require some more time. We can also use EditorialBot (our bot) to set automatic reminders if you know you'll be away for a known period of time.

Please feel free to ping me (@Nikoleta-v3) if you have any questions/concerns. 😄 🙋🏻

mrazomej commented 5 months ago

Review checklist for @mrazomej

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

mrazomej commented 5 months ago

@Nikoleta-v3,

I have a question about the review process. I am also tagging the author (@slabarthe) to get their input.

I am trying to understand the objectives of the software, but it is proving challenging. Although I acknowledge that this is not my field, I should have enough background in partial differential equations and inverse problems to follow what is being done. However, it seems to me that without reading the author's previous publication (which I'm happy to read if necessary), the documentation of the software and the JOSS paper do not contain enough information to follow the point for the software package.

As the JOSS guidelines indicate:

Documentation

There should be sufficient documentation for you, the reviewer, to understand the core functionality of the software under review. A high-level overview of this documentation should be included in a README file (or equivalent).

Furthermore, downloading the worked examples from an external Zenodo link rather than having the relevant information clearly explained in the documentation makes the entry barrier significantly larger. If this is not a problem with how JOSS operates, I am happy to continue the review process as is. However, as a reviewer, I feel that I have to review the relevant publication and carefully read the previous paper since the documentation is not self-contained. I might be completely off with my criteria, and I am happy to change my expectations of what to expect in the documentation.

slabarthe commented 5 months ago

Hi @mrazomej First of all, thank you very much for your time in improving our publication. This is also our first submission to JOSS and we are not sure of our interpretation of the requirements described in the JOSS guidelines. The core functionality of the software is to solve an inverse problem related to the propagation of a chemical compound (namely pheromones) in an agricultural landscape. The objective is to get an early localization of the pheromone emitters, i.e. insect pests, in a crop protection context. The main innovation of the method and software we provide is to integrate prior biological knowledge into the inverse problem, by means of regularization terms. This high-level description is included in the README and the documentation. A mathematical description of this inverse problem and of the biology-informed regularization terms can be found in the JOSS paper. At present, this description is not included in the documentation : we would be happy to include it if you find this information usefull to get a better understanding of the core functionalities.
Another entry point into the core functionalities in the documentation can be found in the src section of the documentation. Four packages can be found here : one for the direct problem, one for the inverse problem, one for the cli and one for plotting functions. If you think it is useful, these packages could be described in the first page of the documentation.

Furthermore, downloading the worked examples from an external Zenodo link rather than having the relevant information clearly explained in the documentation makes the entry barrier significantly larger.

We have chosen to completely separate the package containing the core computational tools and the repository containing the worked example where the different functionalities are presented. This choice was made because the worked example is related to another publication than this JOSS article, whereas the JOSS paper is dedicated to the core functionalities (computational tools to solve the direct and inverse problems). The worked example covers the main functionalities of the software. To speed up computations, the example involves some pre-computed results, which are also fully reproducible. This example allows to reproduce all the figures included in our previous publication. We felt that this worked example was too large to be included in the pherosensor package: this is the main reason why we provided it as a separate repository. Again, we would be happy to include this example in the documentation if you think it would be useful. @Nikoleta-v3 : we would be glad to have your inputs and advises that we will follow has accurately as we can.

Nikoleta-v3 commented 5 months ago

Hello both :wave: (@slabarthe & @mrazomej),

Thank you for your patience. I also needed some time to review the submission. I agree with mrazomej that "the documentation of the software and the JOSS paper do not contain enough information to follow the point of the software package," and this is an issue.

A user should be able to visit the package's repository and documentation and find sufficient guidelines and information on the purpose and functionality of the project. In the current version of the repository, the ReadTheDocs documentation, or even in the paper, there is not a single line of example code to show the user how to use the package.

Examples folder from previous publication

Regarding the examples folder, I believe it's okay that it is a separate thing because, in my understanding, these are files coded together for your previous publications, but they do not suffice as documentation.

Here: https://forgemia.inra.fr/pherosensor/companion-code-bi-da, in the README you have included some information on each of the scripts, but for a user to fully understand the usage of pherosensor, they need to open the scripts and understand what functions/classes are coming from pherosensor. Moreover, there is no real explanation of each line. If users are coming to these scripts after reading the publication it might not be an issue, but if someone is a new user of pherosensor it creates a significant entry barrier.

src section

Another entry point into the core functionalities in the documentation can be found in the src section.

It's great that this list of classes/functions exists! But even there, the documentation could be richer, there is no context as to when each function/class should be used, nor are there docstring examples.

Recommendation

Therefore, the package should be documented better. My recommendation is to create dedicated sections in the documentation for each of the functionalities of the project. There, you should explain the mathematics, followed by showing the code examples to run these functionalities, explaining the input arguments and the outputs. I know it can sound a bit overkill 😅 but it needs to be user-friendly if you are expecting people to use the software.

Here are a few examples of recent JOSS publications that I edited. Maybe they can give you a few ideas about how to structure your online documentation:

These are my personal recommendations. Maybe @mrazomej also has better ones.

slabarthe commented 5 months ago

Thank you @Nikoleta-v3 for these clear recommendations. We will tackle them as soon as possible and get back to you when it is done.

Nikoleta-v3 commented 4 months ago

Thank you @slabarthe. Please feel free to ping me here if you have any questions 😄

Nikoleta-v3 commented 4 months ago

@Shreyas911 did you have a chance to look over the submission? 😄

Shreyas911 commented 4 months ago

Hi @Nikoleta-v3, I plan to have a look before the start of August. Also, based on the comments above, it seems there is some documentation left to be completed/updated. I was wondering if I should hold on until that is complete to provide the best possible review or just go on and review in the next 10 days. Appreciate any advice. :)

Thank you!

Nikoleta-v3 commented 4 months ago

Thank you for your response @Shreyas911! You are right; there will be quite a bit of re-writing of the documentation, so it might be beneficial for you and the authors to wait until that is done. This way, you can provide feedback on the new sections and hopefully test the full functionality of the package. Let's hold off until then (I will ping you then). Thank you! 😄

slabarthe commented 4 months ago

Hi @Nikoleta-v3 , @Shreyas911 , @mrazomej I wanted to let you know that we just finished a major update of the documentation, as suggested by Nikoleta-v3.

  1. We deeply enhanced the documentation of the package itself by adding mathematical description in all the functions and class methods involving a mathematical model. We also enhanced input and output descriptions. (see section "src" of the documentation)
  2. We added a folder "tutorials" at the repository root containing notebook tutorials for the two main core functionalities of the package :
    • solving a reaction-diffusion-convection PDE model describing pheromone propagation in an agricultural landscape (direct problem).
    • inferring the position of pheromone-emitting insects using a biology-informed data assimilation problem from time-series obtained with pheromone sensors (inverse problem). The two notebooks contain a complete hand-on for a simplified toy model, including a precise mathematical description of the problem, and commented usages of the different classes and functions needed to solve the direct and inverse problems.
  3. We added a folder "tutorials/test_numerical_scheme" containing a collection of notebook describing and assessing the numerical schemes used for the main ingredients of the direct and inverse problems : diffusion, convection, adjoint operators, gradient of the loss function...
  4. The notebooks were included as notebook collections in the section "usage" of the documentation

We hope these improvements meet your expectations. We will be happy to make further changes if you consider it necessary.

Shreyas911 commented 3 months ago

Review checklist for @Shreyas911

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

mrazomej commented 3 months ago

@slabarthe, the tutorial in this link has many equations that are not correctly rendered (see image attached). I tried using two browsers (Safari and Brave) and found the same problem in both. Could you fix that?

Screenshot 2024-08-21 at 2 50 17 PM
mrazomej commented 3 months ago

@slabarthe ,

The article has a typo in line 46. There is an unnecessary "$" sign; see the attached image.

Also, out of curiosity, why for j and \tau you use _{obs}, but for m you use ^{obs}?

Screenshot 2024-08-21 at 2 56 18 PM
mrazomej commented 3 months ago

Another question about the paper. Why is the \nabla operator in line 56 presented as \nabla_s? If j_{obs} only depends on s, this seems redundant.

Screenshot 2024-08-21 at 3 00 57 PM
slabarthe commented 3 months ago

Dear @mrazomej First of all, thank you for your comments and suggestions. Hereafter some answers:

Shreyas911 commented 3 months ago

@Nikoleta-v3, I noticed the code is hosted on INRIA servers. It seems far-fetched for COI, but I have collaborators at INRIA who are unrelated to this work.

Nikoleta-v3 commented 3 months ago

Thank you very much for letting me know @Shreyas911 😄 I hadn't even noticed. It's okay, I don't see an issue, but again, thanks for letting me know. 🙏🏻 👍🏻

Shreyas911 commented 3 months ago

@slabarthe, very interesting work. Some comments:

Software paper State of the field: Do the authors describe how this software compares to other commonly-used packages?

Software paper A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?

Software paper Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?

slabarthe commented 2 months ago

Hi @Shreyas911

Thank you very much for your careful review and your positive comments. I have update the paper and the repository according to your advice:

  • A small section on the current state of the field might be needed and help provide further context.

I modified the statement of need to clearly separate a first sections that includes a short description of general features of state-of-the-art packages for data assimilation, which are generalist packages dedicated to the deployment of data assimilation in general contexts. By opposition, the pherosensor-toolbox is a context-specific application-oriented package for crop epidemiosurveillance.

  • The target audience is explicitly mentioned in the documentation but not in the Statement of Need section of the paper.

I added the following sentence at the end of the Statement of Need section: "The target audience is then academic researchers interested in epidemiosurveillance for crops."

The typos have been corrected.

All the suggestions have been included in the paper. Thank you very much again for this work.

Sorry for that : the version of the notebook pulled on the repository was not correctly executed. It has been corrected as you can check here : https://pherosensor-toolbox.readthedocs.io/en/latest/tutorials/pheromone_propagation_model.html

  • Generic suggestion: A readthedocs badge could be added to the README of your git repo.

As suggested, I added a badge for the readthedocs documentation. I also added a badge for the version and the JOSS paper. See documentation and README.

Shreyas911 commented 2 months ago

@editorialbot generate pdf

editorialbot commented 2 months ago

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

Shreyas911 commented 2 months ago

Thanks @slabarthe, this looks good, just a few more typos as shown here: https://docs.google.com/document/d/1CFswBTK_0jr1KNuynJLdH-y6EvMMbRJcdj_29UgnBJs/edit?usp=sharing

Once this is done, I am done with my review and I recommend that the paper be accepted.

slabarthe commented 2 months ago

Hi @Shreyas911 The typos have been corrected. Thank you very much for this swift and efficient review.

@mrazomej @Nikoleta-v3 : we will be happy to further improve our submission if needed.

Nikoleta-v3 commented 2 months ago

Let's wait to see what mrazomej says, and then I'll have one last quick look over the submission 😄 👍🏻

mrazomej commented 2 months ago

I sincerely apologize for the enormous delay. I was traveling with limited internet access. I completed my checklist, and I approved the submission.

slabarthe commented 2 months ago

Thanks a lot @mrazomej for your review and for the great improvements you triggered, together with @Nikoleta-v3 and Shreyas911.

Nikoleta-v3 commented 2 months ago

Thank you all for your time and efforts! @slabarthe I will take one final look in the next few days, and then we can move forward with the submission!

Nikoleta-v3 commented 2 months ago

@slabarthe, thank you very much for your patience! I had some time to review the submission, and I just have a few minor comments at this point.

Paper

(-) Line 32: should it be epidemio surveillance? A space is missing. (-) Could you please ;ower case the reference: PDAF - THE PARALLEL DATA ASSIMILATION FRAMEWORK: EXPERIENCES WITH KALMAN FILTERING to PDAF-the parallel data assimilation framework: experiences with Kalman filtering.

Documentation (-) Here: https://pherosensor-toolbox.readthedocs.io/en/latest/readme.html in the section documentation there is a broken hyperlink.

At this point could you also please:

I can then move forward with accepting the submission 😄

slabarthe commented 2 months ago

Hi @Nikoleta-v3 Thank you so much for all this reviewing process.

I fixed the issues you pointed at in the paper and documentation.

I made a new tagged version of the code (version v0.1.2 ).

I archived the reviewed software in Zenodo.

I checked the metadata. The title matches the paper title, so as the author list.

The DOI of the archive version is https://doi.org/10.5281/zenodo.13834137

slabarthe commented 2 months ago

Hi @Nikoleta-v3 , @mrazomej , @Shreyas911

I just wanted to thank you again very much for this review process and for the valuable advice you provided. It was my first experience with JOSS and I really enjoyed the interactive reviewing procedure that let the authors understand the expectations and the needs for improvements.

Nikoleta-v3 commented 2 months ago

v0.0.8 sorry that was a typo 😆

Nikoleta-v3 commented 2 months ago

@editorialbot generate pdf

Nikoleta-v3 commented 2 months ago

@editorialbot check references

editorialbot commented 2 months ago

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

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

✅ OK DOIs

- 10.1126/science.1183899 is OK
- 10.5281/ZENODO.11506617 is OK
- 10.1142/9789812701831_0006 is OK
- 10.1017/S0021859605005708 is OK
- 10.21105/joss.05150 is OK
- 10.1016/j.envsoft.2014.02.008 is OK

🟡 SKIP DOIs

- None

❌ MISSING DOIs

- None

❌ INVALID DOIs

- None
Nikoleta-v3 commented 2 months ago

@editorialbot set 10.5281/zenodo.13834137 as archive

editorialbot commented 2 months ago

Done! archive is now 10.5281/zenodo.13834137

Nikoleta-v3 commented 2 months ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

Nikoleta-v3 commented 2 months ago

@editorialbot set v0.1.2 as version

editorialbot commented 2 months ago

Done! version is now v0.1.2

Nikoleta-v3 commented 2 months ago

@editorialbot generate pdf

Nikoleta-v3 commented 2 months ago

@editorialbot check references

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

✅ OK DOIs

- 10.1126/science.1183899 is OK
- 10.5281/ZENODO.11506617 is OK
- 10.1142/9789812701831_0006 is OK
- 10.1017/S0021859605005708 is OK
- 10.21105/joss.05150 is OK
- 10.1016/j.envsoft.2014.02.008 is OK

🟡 SKIP DOIs

- None

❌ MISSING DOIs

- None

❌ INVALID DOIs

- None