search

openjournals / joss-reviews

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

[REVIEW]: GOBLIN Lite: A National Land Balance Model for Assessment of Climate Mitigation Pathways for Ireland #6732

Closed editorialbot closed 2 months ago

editorialbot commented 6 months ago

Submitting author: !--author-handle-->@colmduff<!--end-author-handle-- (Colm Duffy) Repository: https://github.com/GOBLIN-Proj/goblin_lite Branch with paper.md (empty if default branch): Version: v0.4.2 Editor: !--editor-->@mengqi-z<!--end-editor-- Reviewers: @david-yannick, @varsha2509 Archive: 10.5281/zenodo.13239928

Status

status

Status badge code:

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

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

@david-yannick & @varsha2509, 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 @mengqi-z 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 @david-yannick

📝 Checklist for @varsha2509

editorialbot commented 6 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 6 months ago

Checking the BibTeX entries failed with the following error: 

Failed to parse BibTeX on value "b" (NAME) ["@", #<BibTeX::Entry >, {:url=>["https://gmd.copernicus.org/articles/15/2239/2022/"], :archiveprefix=>["gmd"], :author=>["{Duffy}, C. and {Prudhomme}, R. and {Duffy}, B. and {Gibbons}, J. and {O'Donoghue}, C. and {Ryan}, M. and {Styles}, D."], :journal=>["Geoscientific Model Development"], :month=>[:mar], :title=>["{GOBLIN version 1.0: a land balance model to identify national agriculture and land use pathways to climate neutrality via backcasting}"], :year=>"2022"}]
editorialbot commented 6 months ago

Software report:

github.com/AlDanial/cloc v 1.90  T=0.19 s (887.3 files/s, 347288.4 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
CSV                             33              0              0          27706
HTML                            50           2848            147          16494
Python                          26           1371           2236           3030
SVG                              1              0              0           2671
CSS                              6            754             63           2549
JavaScript                      12            131            221            880
JSON                             3              0              0            315
Markdown                         7            110              0            313
reStructuredText                24           1787           1988            133
YAML                             3             13              4             69
TeX                              1              0              0             67
Jupyter Notebook                 1              0            507             47
TOML                             1              2              0             27
DOS Batch                        1              8              1             26
make                             1              4              7              9
-------------------------------------------------------------------------------
SUM:                           170           7028           5174          54336
-------------------------------------------------------------------------------

Commit count by author:

    48  Colm Duffy
    32  Colm
editorialbot commented 6 months ago

Paper file info:

📄 Wordcount for paper.md is 899

✅ The paper includes a Statement of need section

editorialbot commented 6 months ago

License info:

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

editorialbot commented 6 months ago

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

mengqi-z commented 6 months ago

👋 @colmduff, @david-yannick, and @varsha2509, Welcome to the review thread for the paper. All communication regarding this submission will take place here.

Please start by reading the "Reviewer instructions & questions" in the first comment above.

Reviewers, please create your checklists outlining JOSS requirements. As you assess the submission, mark any items you believe have been satisfied. Additionally, refer to the JOSS reviewer guidelines linked at the top of this thread.

Our aim is to collaborate with authors to help them meet our criteria. Reviewers are encouraged to submit issues and pull requests directly on the software repository. When doing so, please tag https://github.com/openjournals/joss-reviews/issues/6732 in the issue to create a link to this thread, enabling easy tracking. Please feel free to post comments, questions, and suggestions as they arise, rather than waiting until the entire package is reviewed.

We target completing reviews within 4-6 weeks, but please initiate your review well in advance. JOSS reviews are iterative, and your early feedback will help us stay on schedule.

colmduff commented 6 months ago

@mengqi-z, it doesn't seem to when I generate the paper. It does link to the sources correctly, but its very difficult to tell which paper is which within the text. The pdf does seem to be outputting correctly now, so I am not sure if its still causing an issue? It is a bit strange, as it didn't seem to have an issue with 2020a, just 2020b

mengqi-z commented 6 months ago

@colmduff, The reason it didn't work could be because the author lists from two papers are not exactly the same (In your case, the first four authors are the same, but the rest are not). You could try adding the key field in both entries to distinguish between the papers. There should be a way to address this.

colmduff commented 5 months ago

@mengqi-z, Okie Dokie, I have changed this now. Should be fine I think. I just used the standard formatting and it lists the authors up to the point of departure.

mengqi-z commented 5 months ago

@editorialbot generate pdf

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:

mengqi-z commented 5 months ago

👋 @david-yannick, and @varsha2509, Thank you for reviewing this paper. Could you please update me on your review progress? Thanks!

mengqi-z commented 5 months ago

@david-yannick, @varsha2509 - I'm following up to get a sense of how the reviews are going. Could each of you provide a brief update on your progress? There's no rush, but if you anticipate any delays, please let me know. Thank you!

david-yannick commented 5 months ago

Hi @mengqi-z ! I will be working on it over the next week or so, I have had other projects going on. Sorry for the delayed reply!

varsha2509 commented 5 months ago

Hi @mengqi-z - I'm working on the review and plan to submit mine over the next week. Thank you!

david-yannick commented 5 months ago

Review checklist for @david-yannick

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

varsha2509 commented 4 months ago

Review checklist for @varsha2509

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

david-yannick commented 4 months ago

I am complete with my review, here are a few questions/comments I have regarding the paper

Paper.md

editorialbot commented 4 months ago

Hello @david-yannick, here are the things you can ask me to do:


# List all available commands
@editorialbot commands

# Get a list of all editors's GitHub handles
@editorialbot list editors

# Adds a checklist for the reviewer using this command
@editorialbot generate my checklist

# Set a value for branch
@editorialbot set joss-paper as branch

# Run checks and provide information on the repository and the paper file
@editorialbot check repository

# Check the references of the paper for missing DOIs
@editorialbot check references

# Generates the pdf paper
@editorialbot generate pdf

# Generates a LaTeX preprint file
@editorialbot generate preprint

# Get a link to the complete list of reviewers
@editorialbot list reviewers
colmduff commented 4 months ago

Hi @david-yannick

Thanks a million for your time an effort on this. Apologies for my late response, I have had some revision deadlines on another paper.

To answer your questions:

Thanks again for your time and effort on this, it is greatly appreciated.

colmduff commented 4 months ago

@varsha2509 & @david-yannick

Just a quick note on edits, I will make all changes together once both reviews are in.

Thanks a million!

varsha2509 commented 4 months ago

Hello. I've completed my review and here are my comments. Overall, well written paper and code is well structured. Proposed some comments/suggestions below to improve code readability. Comments on paper:

Minor comments on code:

The authors should consider breaking down run_scenarios into smaller modules to improve readability. For instance, the authors can make use of the ScenarioRunner class and its methods more efficiently: The code for animal data generator could be simplified to look like:

baseline_animal_data, scenario_animal_data, protein_and_milk_summary = self._generate_animal_data(scenario_input_dataframe)

def _generate_animal_data(self, scenario_input_dataframe):
        animal_data_generator = AnimalDataGenerator(
            self.ef_country, self.calibration_year, self.target_year, scenario_input_dataframe
        )
        baseline_animal_data, scenario_animal_data = animal_data_generator.generate_animal_data()
        protein_and_milk_summary = animal_data_generator.generate_livestock_outputs()
        return baseline_animal_data, scenario_animal_data, protein_and_milk_summary

LCA_processing.py - similar comment as above, there seems to be some redundant code in some of the .py files that can be removed.

Sorry for the delay in getting my comments to you. Let me know if you have questions.

colmduff commented 4 months ago

Hi @varsha2509

Thanks a million for your detailed feedback. I will try to repond to the comments here, I will post the combined changes I have made in a seperate post before I resubmit the paper.

To make things a little easier to follow, I am just going to number the response to correspond to the order in which you gave feedback:

  1. Typo fixed
  2. Detailed parameterisation of the CBM CFS3 is discussed in the cited national inventory report, and is beyond the scope of this paper. The model development here is aimed at aligning with national inventory reporting, which is partly why we have integrated the model here. The Teagasc model, as far as I am aware, is a relatively simple stand level model in excel format.
  3. Greater detail added to the caption. These scenarios are arbitrary and meant to illustrate the type of outputs available. I have cited numerous scenario level papers and methodological background to point the reader towards additional resources. I feel that evolving this snap shot overview of the model into more than that is potentially beyond the scope of this paper.
  4. I have made several adjustments here. I have combined everything into a single graphic, which will make things a little bigger. I have also added some contextual clarity in terms of assumptions made with regard to afforestation in the baseline scenario. I have also pointed the user toward additional materials.
  5. We have, in previous work, by default, labeled the baseline scenario as (-1). I will keep this naming format for consistency.
  6. Figure enlarged, additional text added.
  7. Typo in figure corrected
  8. In terms of input validation, there is a minimum level at the moment. We are also working with the Irish Centre for High End Computing, implementing an online simplified version for public use. This will have an online dashboard for inputs, and additional validation will be prioritised as part of the development. At the moment, the tool probably has significant barriers to entry (ability to code) for policymakers. It can assist in generating output to assist in policy decision making, but the average policy maker is unlikley to engage directly.
  9. Added limitations section
  10. This is discussed in cited peer reviewed articles, and is, in my opinion, perhaps beyond the scope here.

Minor comments:

Thank you for taking the time to go through the code in such detail, it is very much appreciated.

I will look to stream line some functions further in the future, this is an ongoing process. With regards to redundant code, I do realise that there is not a specific need to have these variables here. But, it is a personal choice on my part, just to be explicit and seeing variables I am dealing with right in front of me just helps me organise my thoughts a little better. I realise this may offend a developer here and there, but, I am willing to run that risk if it helps me with my workflow.

Thank you again for all your hard work on this, your feedback is very much appreciated.

colmduff commented 4 months ago

Hi @mengqi-z @varsha2509 @david-yannick

Thank you all for your help and engagement with this. Its very much appreciated.

I have responded to each reviewer individually, I just want to summarise the changes to the document here in a consolidated way.

mengqi-z commented 4 months ago

@editorialbot generate pdf

editorialbot commented 4 months ago

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

mengqi-z commented 4 months ago

👋 @varsha2509 @david-yannick - It looks like we are making great progress! Could you provide a brief update in this thread on how things are going? Did @colmduff address all your comments? If so, please make sure to check off the items that are satisfied and let me know your decisions on the paper. Thank you!

colmduff commented 3 months ago

Hi @mengqi-z, @varsha2509, @david-yannick. I hope you are all well. We have been working on the National Inventory updates for several weeks now. The updated emissions factors and land area classifications for 2024 have a significant impact on organic soils under grassland, and wetland emissions. Essentially, the reclassification of grassland means that emissions there are reduced given a lower emissions factor and the fact that a large proportion is now classed as naturally rewetted rather than drained.

Regarding wetlands, revised emisson factors here mean that wetlands are a significantly higher source, as newly incorporated research indicates that no wetland is in a natural state, all have been impacted to by athropogenic activity to some degree.

Regeneration of the data using the updated National Inventory emissions factors and classifications for 2024 are the only changes made.

I have added some national level emissions outputs below. For land cover emisisons and agricutlure emissions. Inputs are generated for the goblin model based on 2024 inputs, but these are not exact, some aggregation, with regards to fertiliser etc. is required. But the outputs serve to illustrate GOBLIN's capacity to replicate the 2024 national inventory.

landuse_emission_validation livestock_emission_validation

david-yannick commented 3 months ago

@mengqi-z My recommendation is to accept

mengqi-z commented 3 months ago

👋 @varsha2509 - Just touching base to see how the review process is going. Have the improvements made by @colmduff addressed all your comments? Please ensure to check off any items that have been resolved from the list. Once you've completed your review, could you please let me know your decision on the paper? Thank you!

varsha2509 commented 3 months ago

Hello there. Yes, I've completed my review and the improvements that @colmduff have made look great. Recommending accepting this work.

mengqi-z commented 3 months ago

@david-yannick @varsha2509 Thank you both for your constructive review!

@colmduff I will make my pass on the paper over the next few days and provide next steps.

mengqi-z commented 3 months ago

@editorialbot generate pdf

editorialbot commented 3 months ago

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

mengqi-z commented 3 months ago

@editorialbot check references

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

OK DOIs

- None

MISSING DOIs

- 10.5194/gmd-15-2239-2022 may be a valid DOI for title: GOBLIN version 1.0: a land balance model to identi...
- 10.1016/j.ecolmodel.2008.10.018 may be a valid DOI for title: CBM-CFS3: A model of carbon-dynamics in forestry a...
- 10.1038/s43247-024-01275-0 may be a valid DOI for title: Defining national net zero goals is critical for f...
- 10.1038/s41893-022-00946-0 may be a valid DOI for title: Randomized national land management strategies for...
- No DOI given, and none found for title: IRELAND’S NATIONAL INVENTORY REPORT 2022: GREENHOU...
- No DOI given, and none found for title: How much grassland biomass is available in Ireland...
- No DOI given, and none found for title: Teagasc National Farm Survey 2021
- No DOI given, and none found for title: The potential availability of land for afforestati...
- No DOI given, and none found for title: GOBLIN Lite

INVALID DOIs

- None
mengqi-z commented 3 months ago

@colmduff I have proofread the paper and create an issue (https://github.com/GOBLIN-Proj/goblin_lite/issues/49) for some remaining items for you to address in the paper.

After you address my comments, could you please:

Once these steps are completed, I can move forward with accepting the submission.

colmduff commented 3 months ago

@colmduff I have proofread the paper and create an issue (GOBLIN-Proj/goblin_lite#49) for some remaining items for you to address in the paper.

After you address my comments, could you please:

  • [x] Double check authors and affiliations (including ORCIDs).
  • [x] Make a release of the software with the latest changes from the review and post the version number here. This is the version that will be used in the JOSS paper.
  • [x] Archive the released software on Zenodo or a similar service (e.g., figshare, an institutional repository).
  • [x] Make sure that the archive (e.g., on Zenodo) has the correct metadata. This includes the title (which should match the paper title) and the author list (ensure the list is correct and does not include people who only made minor contributions). You may also add the authors' ORCIDs.
  • [x] Make sure that the license listed for the archive is the same as the software license.
  • [x] Please post the DOI of the archived version here.

Once these steps are completed, I can move forward with accepting the submission.

@mengqi-z, thanks for this. I have addressed the issues above, I will make the text edits now.

The files are in "pending" status on zenodo, but I have the doi: 10.5281/zenodo.13239928

The version uploaded to PyPi is: v0.4.2

colmduff commented 2 months ago

@mengqi-z, the issue with zenodo is resolved (needed a zip). goblin_lite_0.4.2

mengqi-z commented 2 months ago

@editorialbot generate pdf

mengqi-z 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.5194/gmd-15-2239-2022 is OK
- 10.1016/j.ecolmodel.2008.10.018 is OK
- 10.1038/s43247-024-01275-0 is OK
- 10.1038/s41893-022-00946-0 is OK

MISSING DOIs

- No DOI given, and none found for title: IRELAND’S NATIONAL INVENTORY REPORT 2022: GREENHOU...
- No DOI given, and none found for title: How much grassland biomass is available in Ireland...
- No DOI given, and none found for title: Teagasc National Farm Survey 2021
- No DOI given, and none found for title: The potential availability of land for afforestati...
- No DOI given, and none found for title: GOBLIN Lite

INVALID DOIs

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

mengqi-z commented 2 months ago

@colmduff - Could you please update the Zenodo metadata to ensure that all authors are listed in the author section, matching the author list in your paper? You can keep the current contributor roles as well if you prefer. Thanks!

colmduff commented 2 months ago

@mengqi-z, I have moved co-authors of the paper from contributors to the producer section. Thanks a million.

mengqi-z commented 2 months ago

@editorialbot set 10.5281/zenodo.13239928 as archive

editorialbot commented 2 months ago

Done! archive is now 10.5281/zenodo.13239928

mengqi-z commented 2 months ago

@editorialbot set v0.4.2 as version

editorialbot commented 2 months ago

Done! version is now v0.4.2