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] Livestock detection using DeepForest #260

Closed
42 tasks
acocac opened this issue Oct 31, 2024 · 24 comments · Fixed by #262
Closed
42 tasks

[REVIEW] Livestock detection using DeepForest #260

acocac opened this issue Oct 31, 2024 · 24 comments · Fixed by #262

Comments

@acocac
Copy link
Member

acocac commented Oct 31, 2024

Notebook Review: Issue #259

Binder

Submitting author: @camappel

Repository: https://github.com/eds-book-gallery/95199651-9e81-4cae-a3a7-66398a9a5f62

Notebook idea issue: #257

Editor: @acocac

Reviewer: @ethanwhite @louisavz

Managing EiC: @acocac

Status

Reviewer instructions & questions

Hi 👋 @ethanwhite @louisavz, please carry out your review in this issue by updating the checklist below.

As a reviewer, you contribute to the technical quality of the content published by our community.

Before the review, EiC checked if the submission fits the minimum requirements.

The quality of the proposed contribution can be assessed through scientific, technical and code criteria.

The reviewer guidelines are available here: https://edsbook.org/publishing/guidelines/guidelines-reviewers.html.
Any questions/concerns please let @acocac know.

Review checklist for @ethanwhite

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide.

Conflict of interest

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Code of conduct an peer-review principles

General checks

  • Notebook: Is the notebook file (notebook.ipynb) part of the notebook repository?
  • Contribution and authorship: Does the author list seem appropriate and complete (full name, affiliation, and GitHub/ORCID handle)?
  • Scope and eligibility: Does the submission contain an original and complete analysis according to the scope of EDS book?

Reproducibility

  • Does the notebook run in a local environment?
  • Does the notebook build and run in binder?
  • Are all data sources openly accessible and properly cited (e.g. with citation to a persistent DOI) in the heading section?

Pedagogy

  • Are the notebook purpose and highlights clear?
  • Does the notebook demonstrate some specific data analysis or visualisation techniques?
  • Is the notebook well documented, using both markdown cells and comments in code cells?
  • Does the conclusion section provide clear and concise final say on the tools, analysis and/or datasets used?
  • Is the notebook narrative well written (it does not require editing for structure, language, or writing quality)?

Ethical

  • Is any linkage of datasets in the notebook unlikely to lead to an increased risk of the personal identification of individuals?
  • Is the notebook truthful and clear about any limitations of the analysis (and potential biases in data and/or tools)?
  • Is the notebook unlikely to lead to negative social outcomes, such as (but not limited to) increasing discrimination or injustice?

Other Requirements

  • All mentioned software should be formally and consistently cited wherever possible.
  • Acronyms should be spelled out upon first mention.
  • License conditions on images and figures must be respected (Creative Commons, etc.).

Final approval (post-review)

  • Authors has responded to my review and made changes to my satisfaction. I recommend approving the notebook for publication.

Review checklist for @louisavz

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide.

Conflict of interest

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Code of conduct an peer-review principles

General checks

  • Notebook: Is the notebook file (notebook.ipynb) part of the notebook repository?
  • Contribution and authorship: Does the author list seem appropriate and complete (full name, affiliation, and GitHub/ORCID handle)?
  • Scope and eligibility: Does the submission contain an original and complete analysis according to the scope of EDS book?

Reproducibility

  • Does the notebook run in a local environment?
  • Does the notebook build and run in binder?
  • Are all data sources openly accessible and properly cited (e.g. with citation to a persistent DOI) in the heading section?

Pedagogy

  • Are the notebook purpose and highlights clear?
  • Does the notebook demonstrate some specific data analysis or visualisation techniques?
  • Is the notebook well documented, using both markdown cells and comments in code cells?
  • Does the conclusion section provide clear and concise final say on the tools, analysis and/or datasets used?
  • Is the notebook narrative well written (it does not require editing for structure, language, or writing quality)?

Ethical

  • Is any linkage of datasets in the notebook unlikely to lead to an increased risk of the personal identification of individuals?
  • Is the notebook truthful and clear about any limitations of the analysis (and potential biases in data and/or tools)?
  • Is the notebook unlikely to lead to negative social outcomes, such as (but not limited to) increasing discrimination or injustice?

Other Requirements

  • All mentioned software should be formally and consistently cited wherever possible.
  • Acronyms should be spelled out upon first mention.
  • License conditions on images and figures must be respected (Creative Commons, etc.).

Final approval (post-review)

  • Authors has responded to my review and made changes to my satisfaction. I recommend approving the notebook for publication.

Additional instructions

Reviewer general comments are welcome on this REVIEW issue or directly to the notebook repository.

If you do the latter, you will find a Pull Request titled REVIEW where you can carry out the discussion with authors through ReviewNB, a third-party plugin in GitHub for displaying and commenting Jupyter Notebooks (see further details here).

In addition to ReviewNB, we suggest to explore or run the notebook in:

  • Binder (run): Click the Launch Binder button at the top level of this message.
@acocac
Copy link
Member Author

acocac commented Oct 31, 2024

👋 @ethanwhite @louisavz will conduct the review in this issue.

Please read through the above information and let me know if you have any questions about the review process.

Thank you 🙏

@acocac acocac changed the title [REVIEW] [REVIEW] Livestock detection using DeepForest Nov 1, 2024
@acocac
Copy link
Member Author

acocac commented Nov 12, 2024

@ethanwhite @louisavz - I am just checking in on the status of the review. Are there any obstacles or is there anything I can assist you with? Thank you 🙏

@louisavz
Copy link

louisavz commented Nov 15, 2024

Please check off boxes as applicable, and elaborate in comments below.
Your review is not limited to these topics, as described in the reviewer guide

Conflict of interest

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Code of conduct an peer-review principles

General checks

  • Notebook: Is the notebook file part of the PR?
  • Contribution and authorship: Does the author list seem appropriate and complete (full name, affiliation, and GitHub/ORCID handle)?
  • Scope and eligibility: Does the submission contain an original and complete analysis according to the theme selected?

Reproducibility

  • Does the notebook run in a local environment? (yes, but modifications needed, please see comment below)
  • Does the notebook build and run in binder? yes with the second version
  • Are all data sources openly accessible and properly cited (e.g. with citation to a persistent DOI) in the heading section?

Pedagogy

  • Are the notebook purpose and highlights clear?
  • Does the notebook demonstrate some specific data analysis or visualisation techniques?
  • Is the notebook well documented, using both markdown cells and comments in code cells?
  • Does the conclusion section provide clear and concise final say on the tools, analysis and/or datasets used?
  • Is the notebook narrative well written (it does not require editing for structure, language, or writing quality)?

Ethical

  • Is any linkage of datasets in the notebook unlikely to lead to an increased risk of the personal identification of individuals?
  • Is the notebook truthful and clear about any limitations of the analysis (and potential biases in data and/or tools)?
  • Is the notebook unlikely to lead to negative social outcomes, such as (but not limited to) increasing discrimination or injustice?

Other Requirements

  • All mentioned software should be formally and consistently cited wherever possible.
  • Acronyms should be spelled out upon first mention.
  • Licence conditions on images and figures must be respected (Creative Commons, etc.).

Final approval (post-review)

  • Authors has responded to my review and made changes to my satisfaction. I recommend approving the notebook for publication.

@louisavz
Copy link

Hi, after about a dozen attempts, I am still not able to run this notebook on Binder. The kernel keeps restarting at various places.

For local install, I'm running on

Apple M2 Pro, Ventura 13.6.6
Safari browser v17.3.1 
Jupyter notebook

I ran into the following issues and noted the additional steps needed before I could run the notebook locally.

Cell 1 of pip install,

!pip -q install deepforest==1.4.0

I ran into this error:

error: subprocess-exited-with-error
  
  × Getting requirements to build wheel did not run successfully.
  │ exit code: 1
  ╰─> [2 lines of output]
    WARNING:root:Failed to get options via gdal-config: [Errno 2] No such file or directory: 'gdal-config'
    ERROR: A GDAL API version must be specified. Provide a path to gdal-config using a GDAL_CONFIG environment variable or use a GDAL_VERSION environment variable.
    [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: subprocess-exited-with-error

× Getting requirements to build wheel did not run successfully.
│ exit code: 1
╰─> See above for output.

note: This error originates from a subprocess, and is likely not a problem with pip.

I then used

conda install -c conda-forge gdal

to bypass this issue.

At cell 8, I ran into a NotImplementedError. It gives the suggestion:

NotImplementedError: The operator 'torchvision::nms' is not currently implemented for the MPS device. If you want this op to be added in priority during the prototype phase of this feature, please comment on https://github.com/pytorch/pytorch/issues/77764. As a temporary fix, you can set the environment variable PYTORCH_ENABLE_MPS_FALLBACK=1 to use the CPU as a fallback for this op. WARNING: this will be slower than running natively on MPS.

So I used the command

export PYTORCH_ENABLE_MPS_FALLBACK=1

The notebook must be restarted completely (e.g., restarting the kernel was not enough).

@acocac
Copy link
Member Author

acocac commented Nov 18, 2024

@louisavz thanks for going through the notebook and report issues!

The first issue about missing GDAL is now fixed in the latest version of the review branch in the notebook repository.

The workaround for the PyTorch MPS is typing in the console:
export PYTORCH_ENABLE_MPS_FALLBACK=1

Then you launch the Jupyter notebook/lab server eg jupyter notebook or jupyter lab

Could you double check again the installation and successful run in your laptop? 🙏

re Binder. We're trying to figure out the issue. EDS book notebooks also run in a custom binder, but the access is restricted to collaborators or individuals affiliated to EU institutions. The custom binder successfully launches the session so we might need to double check if the error is temporary in the standard Binder.

@acocac
Copy link
Member Author

acocac commented Nov 18, 2024

@ethanwhite, can you still provide a review for this submission? Please let us know if we can do something to help you move forward.

@ethanwhite
Copy link

@acocac - Yes. You said 2-4 weeks so it's on my To Do list with a deadline of 4 weeks, which is November 29th

@acocac
Copy link
Member Author

acocac commented Nov 19, 2024

@ethanwhite - thanks for confirming! Looking forward to supporting/moderating the conversation between you and the author if necessary.

@ethanwhite
Copy link

@acocac - I don't appear to have permission to check boxes in the main checklist

@acocac
Copy link
Member Author

acocac commented Nov 22, 2024

@ethanwhite - thanks for raising this. Can you copy and paste the checklist from the guidelines from reviewers?

@ethanwhite
Copy link

Yep, I already have a copy I'm working on locally and will paste when done

@ethanwhite
Copy link

ethanwhite commented Nov 22, 2024

Checklist for @ethanwhite

Conflict of interest

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Code of conduct an peer-review principles

General checks

  • Notebook: Is the notebook file part of the PR?
  • Contribution and authorship: Does the author list seem appropriate and complete (full name, affiliation, and GitHub/ORCID handle)?
  • Scope and eligibility: Does the submission contain an original and complete analysis according to the theme selected?

Reproducibility

  • Does the notebook run in a local environment?
  • Does the notebook build and run in binder?
  • Are all data sources openly accessible and properly cited (e.g. with citation to a persistent DOI) in the heading section?

Pedagogy

  • Are the notebook purpose and highlights clear?
  • Does the notebook demonstrate some specific data analysis or visualisation techniques?
  • Is the notebook well documented, using both markdown cells and comments in code cells?
  • Does the conclusion section provide clear and concise final say on the tools, analysis and/or datasets used?
  • Is the notebook narrative well written (it does not require editing for structure, language, or writing quality)?

Ethical

  • Is any linkage of datasets in the notebook unlikely to lead to an increased risk of the personal identification of individuals?
  • Is the notebook truthful and clear about any limitations of the analysis (and potential biases in data and/or tools)?
  • Is the notebook unlikely to lead to negative social outcomes, such as (but not limited to) increasing discrimination or injustice?

Other Requirements

  • All mentioned software should be formally and consistently cited wherever possible.
  • Acronyms should be spelled out upon first mention.
  • Licence conditions on images and figures must be respected (Creative Commons, etc.).

Final approval (post-review)

  • Authors has responded to my review and made changes to my satisfaction. I recommend approving the notebook for publication.

@ethanwhite
Copy link

Very nice work @camappel! This is a beautiful example of how we hope that DeepForest will be used. The notebook is easy to follow and covers all of the important points along the way. I love it!

Three small notes:

@acocac
Copy link
Member Author

acocac commented Nov 23, 2024

@camappel could you please address the comments above? Both reviewers argue the notebook doesn't work in the standard Binder. If there isn't a workaround to reduce the memory usage while loading the models, I suggest indicating the limitation within a warning/note block somewhere in the notebook (see for example, https://edsbook.org/notebooks/gallery/3286b92f-4fae-4cc6-a29e-e408bc844542/notebook).

Please also consider addressing the additional observations provided by the reviewers.

Let us know when the changes are implemented and then I'll tag the reviewers for getting their final concept of the submission. Thank you 🙏

@louisavz
Copy link

Hi @acocac,

For the additional step on the workaround for the PyTorch MPS, I recommend updating the README to include this step or as a side note if it is unique to operating systems.

I've tried again cloning the repo and now ran into an import error. It looks to be still GDAL related.

---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
Cell In[2], line 14
     11 import matplotlib.pyplot as plt
     12 from PIL import Image
---> 14 from deepforest import main
     15 from deepforest.visualize import plot_predictions
     16 from huggingface_hub import hf_hub_download

File ~/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/python3.12/site-packages/deepforest/main.py:11
      9 import pandas as pd
     10 import pytorch_lightning as pl
---> 11 import rasterio as rio
     12 import torch
     13 from PIL import Image

File ~/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/python3.12/site-packages/rasterio/__init__.py:25
     22                 if p and glob.glob(os.path.join(p, "gdal*.dll")):
     23                     os.add_dll_directory(os.path.abspath(p))
---> 25 from rasterio._io import Statistics
     26 from rasterio._vsiopener import _opener_registration
     27 from rasterio._show_versions import show_versions

ImportError: dlopen(/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/python3.12/site-packages/rasterio/_io.cpython-312-darwin.so, 0x0002): Library not loaded: @rpath/libgdal.36.dylib
  Referenced from: <219F70AC-702D-38A1-8624-B16863B9C5D5> /Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/python3.12/site-packages/rasterio/_io.cpython-312-darwin.so
  Reason: tried: '/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/libgdal.36.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/libgdal.36.dylib' (no such file), '/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/libgdal.36.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/libgdal.36.dylib' (no such file), '/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/libgdal.36.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/libgdal.36.dylib' (no such file), '/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/libgdal.36.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/libgdal.36.dylib' (no such file), '/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/bin/../lib/libgdal.36.dylib' (no such file), '/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/bin/../lib/libgdal.36.dylib' (no such file), '/usr/local/lib/libgdal.36.dylib' (no such file), '/usr/lib/libgdal.36.dylib' (no such file, not in dyld cache)

@acocac
Copy link
Member Author

acocac commented Nov 27, 2024

@louisavz @ethanwhite - the author of the notebook has addressed your observations. Because of the out-of-memory issues in Binder, the author is proposing a new version. Sharing below two options to inspect the notebook:

Option 1 (via Binder)

Binder

Option 2 (via ReviewNB) - useful to track changes from the initial version
https://app.reviewnb.com/eds-book-gallery/95199651-9e81-4cae-a3a7-66398a9a5f62/pull/5/

After the inspection, I'd appreciate confirming if you're satisfied with the new version and therefore recommend the publication for EDS book.

Thank you 🙏

@acocac
Copy link
Member Author

acocac commented Nov 27, 2024

Hi @acocac,

For the additional step on the workaround for the PyTorch MPS, I recommend updating the README to include this step or as a side note if it is unique to operating systems.

I've tried again cloning the repo and now ran into an import error. It looks to be still GDAL related.

---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
Cell In[2], line 14
     11 import matplotlib.pyplot as plt
     12 from PIL import Image
---> 14 from deepforest import main
     15 from deepforest.visualize import plot_predictions
     16 from huggingface_hub import hf_hub_download

File ~/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/python3.12/site-packages/deepforest/main.py:11
      9 import pandas as pd
     10 import pytorch_lightning as pl
---> 11 import rasterio as rio
     12 import torch
     13 from PIL import Image

File ~/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/python3.12/site-packages/rasterio/__init__.py:25
     22                 if p and glob.glob(os.path.join(p, "gdal*.dll")):
     23                     os.add_dll_directory(os.path.abspath(p))
---> 25 from rasterio._io import Statistics
     26 from rasterio._vsiopener import _opener_registration
     27 from rasterio._show_versions import show_versions

ImportError: dlopen(/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/python3.12/site-packages/rasterio/_io.cpython-312-darwin.so, 0x0002): Library not loaded: @rpath/libgdal.36.dylib
  Referenced from: <219F70AC-702D-38A1-8624-B16863B9C5D5> /Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/python3.12/site-packages/rasterio/_io.cpython-312-darwin.so
  Reason: tried: '/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/libgdal.36.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/libgdal.36.dylib' (no such file), '/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/libgdal.36.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/libgdal.36.dylib' (no such file), '/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/libgdal.36.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/libgdal.36.dylib' (no such file), '/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/libgdal.36.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/lib/libgdal.36.dylib' (no such file), '/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/bin/../lib/libgdal.36.dylib' (no such file), '/Users/lvanzeeland/miniconda3/envs/95199651-9e81-4cae-a3a7-66398a9a5f62/bin/../lib/libgdal.36.dylib' (no such file), '/usr/local/lib/libgdal.36.dylib' (no such file), '/usr/lib/libgdal.36.dylib' (no such file, not in dyld cache)

We'll address this in the post-print stage (after the recommendation). Thanks for the suggestion!

@louisavz
Copy link

@acocac The new version of the notebook on Binder runs for me. Thank you for uploading it. Meanwhile, do you want me to do anything further to make the notebook run locally? I'm not sure if you meant you will address the suggestion in the post-print stage, or both the suggestion and the issue with GDAL & rasterio.

@acocac
Copy link
Member Author

acocac commented Nov 28, 2024

@acocac The new version of the notebook on Binder runs for me. Thank you for uploading it. Meanwhile, do you want me to do anything further to make the notebook run locally? I'm not sure if you meant you will address the suggestion in the post-print stage, or both the suggestion and the issue with GDAL & rasterio.

Great to hear the notebook on Binder runs for you!

re run locally. I've worked for me to run the notebook in my Mac M2. Following the guidelines of DeepForest, I suggest forcing the installation of geopandas after setting the computational environment using environment.yml.

All notebook repositories in EDS book include the necessary installation steps. The README of the submission include now a special note for Mac M1/M2 users, please refer to the files changed within the REVIEW PR.

Please let us know if the notebook runs with the suggested instructions. 🤞

@louisavz
Copy link

Hi @acocac, thanks for pointing out the readme in the review branch. Apologies, I pulled the review branch but did not open the readme page for the updated instruction (I thought it was included in the new environment.yml file). I've started fresh and the notebook runs locally for me now. Thank you!

@acocac
Copy link
Member Author

acocac commented Dec 1, 2024

@ethanwhite - this is a kindly reminder to revisit the checklist and confirm if you recommend the publication of the new version for EDS book. Thank you 🙏

@ethanwhite
Copy link

Looks great! All boxes checked!

@acocac
Copy link
Member Author

acocac commented Dec 2, 2024

@camappel - Congratulations, your notebook is recommended for publication! 🚀

Huge thanks to the reviewers: @louisavz @ethanwhite — your contributions make the publication of the notebook possible 🙏

Next steps (optional for reviewers): @camappel - I'll contact you for validating the post-print version and confirm a suitable date to announce the publication among the communication channels of the EDS book community.

@camappel
Copy link

camappel commented Dec 2, 2024

Brilliant. Thanks to everyone for their time/support so far!

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

Successfully merging a pull request may close this issue.

4 participants