Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
editorialbot opened this issue May 8, 2024 · 28 comments
Assignees
Labels

Comments

@editorialbot
Copy link
Collaborator

editorialbot commented May 8, 2024

Submitting author: @colmduff (Colm Duffy)
Repository: https://github.com/GOBLIN-Proj/goblin_lite
Branch with paper.md (empty if default branch):
Version: v0.3.4
Editor: @mengqi-z
Reviewers: @david-yannick, @varsha2509
Archive: Pending

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

Paper file info:

📄 Wordcount for paper.md is 899

✅ The paper includes a Statement of need section

@editorialbot
Copy link
Collaborator Author

License info:

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

@editorialbot
Copy link
Collaborator Author

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@mengqi-z
Copy link

mengqi-z commented May 8, 2024

👋 @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 #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
Copy link

colmduff commented May 8, 2024

@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
Copy link

mengqi-z commented May 8, 2024

@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
Copy link

colmduff commented May 9, 2024

@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
Copy link

mengqi-z commented May 9, 2024

@editorialbot generate pdf

@editorialbot
Copy link
Collaborator Author

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@mengqi-z
Copy link

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

@mengqi-z
Copy link

@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
Copy link

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
Copy link

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

@david-yannick
Copy link

david-yannick commented Jun 3, 2024

Review checklist for @david-yannick

Conflict of interest

  • I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

General checks

  • Repository: Is the source code for this software available at the https://github.com/GOBLIN-Proj/goblin_lite?
  • License: Does the repository contain a plain-text LICENSE or COPYING file with the contents of an OSI approved software license?
  • Contribution and authorship: Has the submitting author (@colmduff) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  • Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
  • Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
  • Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
  • Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
  • 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

Software paper

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • 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?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • 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?

@varsha2509
Copy link

varsha2509 commented Jun 10, 2024

Review checklist for @varsha2509

Conflict of interest

  • I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

General checks

  • Repository: Is the source code for this software available at the https://github.com/GOBLIN-Proj/goblin_lite?
  • License: Does the repository contain a plain-text LICENSE or COPYING file with the contents of an OSI approved software license?
  • Contribution and authorship: Has the submitting author (@colmduff) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  • Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
  • Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
  • Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
  • Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
  • 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

Software paper

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • 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?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • 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?

@david-yannick
Copy link

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

Paper.md

  • Overall, software paper is concise and well written.
  • Based on previous work cited utilizing the original GOBLIN model [lines 24-25], which also has been used in several studies in the AFOLU sector [lines 30-32]. This is specialized to work in this particular need.
  • Are calculated fluxes similar to a micrometeorological/atmospheric sign convention? Ecosystem CO2 uptake/photosynthesis a negative flux, and fluxes released to atmosphere are positive. It may be helpful to state.
  • Figure 2 – Croplands not represented in example? It is mentioned that there is a reduction in livestock being converted to forest [50-51]. Were livestock lands considered croplands in this example or perhaps not labeled/represented in figure?
  • Line 21 CH4 formatting needs to be addressed

@editorialbot
Copy link
Collaborator Author

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
Copy link

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:

  • calculated fluxes: Yes, emission == positive, and removal == negative
  • Figure 2: Croplands actually are in there, they are just super small. Crop area in ireland is < 5% of total agricultural area, so the sector is very heavily dominated by livestock production.
  • Formatting: Good catch, my brain may have filtered that out when I was proof reading.

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

@colmduff
Copy link

@varsha2509 & @david-yannick

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

Thanks a million!

@varsha2509
Copy link

varsha2509 commented Jun 19, 2024

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:

  • Line 21 - typo after CH4, remove _
  • Line 48 - Looking at the CMB-CSF3 paper, the authors mention that the flexible nature of the model has enabled it to be adapted for use in several other countries. Are there any limitations that users should be aware of for the use of this model within Irish context? Came across this tool (https://www.teagasc.ie/crops/forestry/advice/environment/forest-carbon-tool/) that may be specific to IE. Is this something the authors have considered?
  • Figure 2 -Can the authors add more description in the caption explaining each of those figures? Can the authors also please add a little bit more description on what Scenario 0 and 1 indicate? I see the information provided in config.json, but what do these two scenarios indicate?
  • Figure 3 - The figure is quite small and hard to read. Can the authors make the figure larger and provide more information on what the results mean? For instance, under the baseline, we see an increase in carbon removal by above ground biomass between ~2030 - 2040, but carbon removal by ABG reduces overtime for scenario 0 and 1. Why is that?
  • Recommend renaming scenarios to base something like Baseilne (scenario 0), low upland sheep population or something along those lines for scenario 1, high upland sheep population for scenario 2.
  • Figure 4 - please enlarge the figure and include more description about the results (even if the results are for illustrative purposes).
  • Figure 4 - Scenarios are mapped as scenario 0, 1, 2 - these should be -1, 0 and 1?
  • In lines 67 - 72, authors mentioned that care must be taken while defining inputs. What checks/bounds have the authors added to the model in order to validate the inputs and outputs produced? If this tool is designed for use in policy decisions, having some validation can go a long way. Is this something authors will consider in future iterations of the tool?
  • Can authors please include the limitations of this work so that users of this tool are aware of limitations?
  • Can the authors also talk about how this library compares against other software-packages, if available?

Minor comments on code:

  • goblin.py - There seems to be some redundant code here specifically lines 111 - 115. The reassignment of instance variables to local variables is not necessary here. When referring to instance variables in run_scenarios, authors can use self. to access them instead.

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
Copy link

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
Copy link

colmduff commented Jun 20, 2024

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.

  • Typo in line 21 corrected
  • Fig 2 has been edited to include additional detail. Additional supporting detail also added.
  • Fig 3 has also been edit to produce a single, larger figure, with additional supporting detail added. I also changed the harvesting rate in standing forest from 10% to 50% for a slightly more realistic looking output.
  • Fig 4, additional detail added. However, I have tried to enlarge this figure, but it seems to be at the maximum allowable. This is perhaps due to the width of the figure itself. It is still readable at a moderate zoom level. I can look at rendering an additional row of subplots, rather than a single row. Perhaps that might help, if the current format is not feasible.
  • Typo in Figure 4 corrected
  • Limitations section has been added.

@mengqi-z
Copy link

@editorialbot generate pdf

@editorialbot
Copy link
Collaborator Author

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@mengqi-z
Copy link

👋 @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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants