openjournals / joss-reviews

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

[REVIEW]: A Module for Calibrating Impact Functions in the Climate Risk Modeling Platform CLIMADA #6755

Open editorialbot opened 1 month ago

editorialbot commented 1 month ago

Submitting author: !--author-handle-->@peanutfun<!--end-author-handle-- (Lukas Riedel) Repository: https://github.com/CLIMADA-project/climada_python Branch with paper.md (empty if default branch): calibrate-impact-functions Version: v5 Editor: !--editor-->@observingClouds<!--end-editor-- Reviewers: @sunt05, @naingeet Archive: Pending

Status

status

Status badge code:

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

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

@sunt05 & @naingeet, 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 @observingClouds 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 @sunt05

editorialbot commented 1 month 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 month ago

Software report:

github.com/AlDanial/cloc v 1.90  T=0.46 s (512.9 files/s, 234364.2 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                         137           8499          18766          32640
CSV                             14              2              0           3462
Jupyter Notebook                38              0          36741           3082
Markdown                        10            248              0            737
reStructuredText                20            418            411            523
CSS                              1             91             11            405
TeX                              2             28              0            366
YAML                             6             26             26            207
make                             2             29              7            108
Bourne Shell                     3             20              1             44
Dockerfile                       1              7              8             15
-------------------------------------------------------------------------------
SUM:                           234           9368          55971          41589
-------------------------------------------------------------------------------

Commit count by author:

  1008  gabriela
   394  sameberenz
   342  emanuel-schmid
   283  Thomas Vogt
   186  Carmen Steinmann
   167  schmide
   109  Lukas Riedel
    94  Jan Hartman
    93  ThomasRoosli
    81  Evelyn-M
    73  ingajsa
    69  Chahan Kropf
    63  samluethi
    57  Emanuel Schmid
    56  Gabriela Aznar
    51  Yue Yu
    48  Benoît Guillod
    42  zeliest
    40  David N. Bresch
    40  frqqyy
    32  Chahan M. Kropf
    29  Samuel Lüthi
    29  aleciu
    29  gabrielaznar
    27  Chris Fairless
    23  timschmi95
    19  Marine Perus
    18  Samuel Eberenz
    17  Sam Luethi
    16  carmensteinmann
    15  manniepmkam
    14  Nicolas Colombi
    14  Simona Meiler
    13  wjan262
    10  climada
     9  Unknown
     9  aleeciu
     8  Thomas Röösli
     8  Zélie Stalhandske
     7  Carmen B. Steinmann
     6  Samuel Juhel
     6  leonie-villiger
     5  Alessio Ciullo
     5  NicolasColombi
     4  KasparTo
     4  Pui Man (Mannie) Kam
     4  Rachel_B
     4  Tobias Geiger
     4  climada.ethz.ch
     3  Evelyn Mühlhofer
     3  davidnbresch
     2  Benoit P. Guillod
     2  Sarah Hülsen
     2  Timo Schmid
     2  Zelie Stalhandske
     2  raphael-portmann
     2  veronicabozzini
     1  DarioStocker
     1  Kam Lam Yeung
     1  Mannie Kam
     1  Rachel Bungerer
     1  Schmid  Timo
     1  luseverin
     1  scem
     1  simonameiler
editorialbot commented 1 month ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.5194/gmd-12-3085-2019 is OK
- 10.5194/gmd-14-351-2021 is OK
- 10.1038/s41592-019-0686-2 is OK
- 10.5194/nhess-21-393-2021 is OK
- 10.1016/j.firesaf.2015.04.008 is OK
- 10.5194/gmd-2021-192 is OK
- 10.5281/zenodo.8383171 is OK
- 10.5194/egusphere-2024-93 is OK
- 10.21203/rs.3.rs-3682198/v1 is OK
- 10.1002/met.2035 is OK
- 10.5194/nhess-21-279-2021 is OK
- 10.5194/nhess-2023-158 is OK
- 10.1017/CBO9781107415379.016 is OK
- 10.21203/rs.3.rs-3807553/v1 is OK
- 10.1175/2009BAMS2755.1 is OK
- 10.1175/2008MWR2395.1 is OK
- 10.5194/essd-12-817-2020 is OK

MISSING DOIs

- No DOI given, and none found for title: Bayesian Optimization: Open source constrained glo...
- No DOI given, and none found for title: 2022 Disasters in Numbers
- No DOI given, and none found for title: Emergency Events Database (EM-DAT)
- No DOI given, and none found for title: Risk and Uncertainty Assessment for Natural Hazard...
- No DOI given, and none found for title: Oasis Loss Modelling Framework

INVALID DOIs

- None
editorialbot commented 1 month ago

Paper file info:

📄 Wordcount for paper.md is 1091

✅ The paper includes a Statement of need section

editorialbot commented 1 month ago

License info:

🟡 License found: GNU General Public License v3.0 (Check here for OSI approval)

editorialbot commented 1 month ago

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

observingClouds commented 1 month ago

@sunt05, @naingeet thank you very much for agreeing to review this submission. Your expertise is very valuable. With the opening of this issue, the official review process can now start. A good starting point is to create the personal review checklist by commenting @editorialbot generate my checklist and tick the checked items as you go through the submission. You can find more information about his above or also in the documentation.

Please note that we ask to review the additions made via the PR https://github.com/CLIMADA-project/climada_python/pull/692 as they reflect the new additions and the contents of the submitted manuscript (see this comment). You might want to checkout the particular branch calibrate-impact-funcions.

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 openjournals/joss-reviews#6755 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 (@observingclouds) if you have any questions/concerns.

observingClouds commented 1 month ago

@peanutfun thank you for your submission. The review will now start. Please check back regularly and address the reviewers comments and issues as they appear to help us make the review process as interactive and effective as possible. Could you in the meantime try to fix the missing DOIs mentioned above and alternatively provide a URL if a DOI is not available. Thank you!

sunt05 commented 1 month ago

Review checklist for @sunt05

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

peanutfun commented 1 month ago

@editorialbot generate pdf

editorialbot commented 1 month ago

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

peanutfun commented 1 month ago

@editorialbot check references

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

OK DOIs

- 10.5194/gmd-12-3085-2019 is OK
- 10.5194/gmd-14-351-2021 is OK
- 10.1038/s41592-019-0686-2 is OK
- 10.5194/nhess-21-393-2021 is OK
- 10.1016/j.firesaf.2015.04.008 is OK
- 10.5194/gmd-2021-192 is OK
- 10.5281/zenodo.8383171 is OK
- 10.5194/egusphere-2024-93 is OK
- 10.21203/rs.3.rs-3682198/v1 is OK
- 10.1002/met.2035 is OK
- 10.5194/nhess-21-279-2021 is OK
- 10.5194/nhess-2023-158 is OK
- 10.1017/CBO9781107415379.016 is OK
- 10.21203/rs.3.rs-3807553/v1 is OK
- 10.1175/2009BAMS2755.1 is OK
- 10.1175/2008MWR2395.1 is OK
- 10.5194/essd-12-817-2020 is OK

MISSING DOIs

- No DOI given, and none found for title: Bayesian Optimization: Open source constrained glo...
- No DOI given, and none found for title: 2022 Disasters in Numbers
- No DOI given, and none found for title: Emergency Events Database (EM-DAT)
- No DOI given, and none found for title: Risk and Uncertainty Assessment for Natural Hazard...
- No DOI given, and none found for title: Oasis Loss Modelling Framework

INVALID DOIs

- None
peanutfun commented 1 month ago

@observingClouds We were able to replace one work marked with "missing DOIs" with a suitable publication with a DOI. For the rest, we provide URLs:

peanutfun commented 3 weeks ago

@sunt05 @naingeet We are looking forward to receiving your feedback on the code and the paper draft in https://github.com/CLIMADA-project/climada_python/pull/692. If you have any trouble installing CLIMADA or executing the provided tutorial Jupyter script, please let us know.

sunt05 commented 2 weeks ago

Thanks @peanutfun for the nice work!

Could you please address this issue while I was testing your notebook?

sunt05 commented 4 days ago

Congratulations to the CLIMADA team on delivering this excellent work, which I believe will be very useful to the climate risk community.

After trying out the new development, I have only two minor points for the climada team to consider:

Slightly off-topic, while I understand CLIMADA is a comprehensive codebase with various dependencies, I believe the dependency structure could be simplified for easier installation. Even as an experienced Python user and developer, it took me a while to get it working. New users interested in climate risk analysis rather than Python development might struggle even more. Also, since the complexity largely comes from geospatial analysis packages, I'd suggest focusing on climate risk analysis and leaving the geospatial part to more experienced users.

sunt05 commented 4 days ago

@observingClouds Thanks for inviting me to this review. Please see my comments above. Looking forward to future collaborations.

observingClouds commented 3 days ago

@sunt05 thank you very much for your comments. Depending on the response of @peanutfun I might ask you one more time to look through any potential edits.

observingClouds commented 3 days ago

@peanutfun please have a look at the last comments of sunt05. Please improve the demo notebook accordingly and improve the user experience when installing CLIMADA.

I expect the reviews from @naingeet also coming in this week, but please work in the meantime on above's issues to help the interactivity of the review process.

Cheers!

peanutfun commented 3 days ago

@sunt05 Thank you for your review. I am very glad about your positive feedback. We will add a quickstart section to the tutorial, as you suggested, in the next few days.

I understand that the setup for the development version of Climada is quite involved, especially if you have a working setup that does not rely on Conda. Unfortunately, this is a somewhat historical problem we cannot resolve easily. First, we think that geospatial analysis combined with the comparably simple task of impact calculation is the main advantage of Climada, and we would not want to make it optional. Second, the trouble comes from a few C(++)-library dependencies that are used for some submodules of Climada. We plan to move these to the adjacent repository https://github.com/CLIMADA-project/climada_petals, see https://github.com/CLIMADA-project/climada_python/discussions/729. Our goal is to make the "Core" https://github.com/CLIMADA-project/climada_python repository and all its dependencies installable only via Pip, which is currently not the case. However, we decided to update these submodules before moving, as they have grown outdated, and are waiting for feedback from collaborators, see https://github.com/CLIMADA-project/climada_python/discussions/811. Third, our current module import patterns do not allow well for optional dependencies. Also, testing (combinations of) optional dependencies is a larger task that would overburden our current resources.

Finally, we provide a conda-forge package for all Climada releases. We hope this simplifies installation for inexperienced users enough to get started with Climada easily:

conda create -n climada_env climada

Of course, they then cannot switch to a development or feature branch, as you were required to.

I hope this clarifies why we currently do not see an easy, quick way to improve the user experience regarding development version installation. We are well aware of the issue, and resolving it is a somewhat long-term goal for us. Still, if you encountered some pitfalls our installation instructions do not cover, we would gladly incorporate any suggestions for improvement.

observingClouds commented 3 days ago

@peanutfun thank you for your extended reply. Do I understand it correctly that all dependencies will be easily installable via conda once the branch that is the base of this review will be merged?

peanutfun commented 3 days ago

@observingClouds We are merging all our feature branches into an unstable develop branch first. To install all dependencies for feature or develop branches, one has to follow the "advanced" installation instructions(i.e., installation from source) for Climada. In step 4, git checkout develop can be replaced by a feature branch like the one under review.

The calibration module under review will become available in the conda-forge package when we create a new release, merging develop into main. Said package also includes all dependencies. We aim for our next release when this review is concluded. From then on, users following the "simple instructions" will benefit from the new calibration module.