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

Add documentation for distant AOIs for grid generation #55

Merged
merged 10 commits into from
Jul 1, 2022

Conversation

jtmiclat
Copy link
Contributor

@jtmiclat jtmiclat commented Jun 27, 2022

Closes #35
Closes #59
Closes #60

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@jtmiclat jtmiclat changed the title Add documentaion for distance AOI Add documentaion for distanct AOIs for grid generation Jun 27, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jun 27, 2022

Visit the preview URL for this PR (updated for commit 162909d):

https://geowrangler--pr55-docs-document-grid-p-q6ichi76.web.app

(expires Thu, 07 Jul 2022 01:59:27 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@jtmiclat
Copy link
Contributor Author

@joshuacortez
Copy link
Contributor

Hello @jtmiclat , tried to run the colab notebook but encountered an error when loading the admin_grids geojson. It couldn't find the file in the data directory. Thanks!

grid_gdf = gpd.read_file("../data/region3_admin_grids.geojson")

@jtmiclat
Copy link
Contributor Author

jtmiclat commented Jun 27, 2022

@joshuacortez Fixed! Pls, try it out again. Please use this link as the button that opens collab points to master and not this branch https://colab.research.google.com/github/thinkingmachines/geowrangler/blob/docs/document-grid-perfomance/notebooks/tutorial.grids.ipynb

@joshuacortez
Copy link
Contributor

Thanks for the corrections!

I looked at it and the example of having a ~50 square AOIs (divided into large cells of 1km) might not be representative of some actual use cases? Curious about the performance of dividing irregularly shaped polygons across a country into granular cells (e.g. 100m instead of 1km). That's where performance concerns (speed, memory) might be more apparent.

I can help out with this if ever too.

@jtmiclat
Copy link
Contributor Author

@joshuacortez Can you provide some irregular polygons we can try?

@joshuacortez
Copy link
Contributor

Alright, provided test polygon AOIs in this GPKG file

Parameters I used:

  • PHL bounding box as custom boundaries [-236221.24150117, 507465.65086805, 1119703.39754333,
    2336815.97643073] in epsg:3123
  • cell_size of 100 meters

Some benchmarked runtime results using previous code:

  • ~40 sec for the first 100 AOIs
  • ~43 min for all the AOIs

@jtmiclat
Copy link
Contributor Author

jtmiclat commented Jun 30, 2022

@joshuacortez I ran the benchmark on colab (Single core with ~14GB ram). It is very similar to what you mentioned.

Screenshot from 2022-06-30 08-43-22

@tm-kah-alforja tm-kah-alforja mentioned this pull request Jun 30, 2022
9 tasks
@joshuacortez
Copy link
Contributor

joshuacortez commented Jun 30, 2022

Awesome, thanks @jtmiclat! This looks pretty good. I ran it on a local machine with similar specs

@joshuacortez
Copy link
Contributor

I wonder if it makes sense to have a function that takes in a geodataframe of multiple AOIs instead of manually iterating through each AOI and making separate geodataframes. I don't feel too strongly about this but might look "cleaner" from a user perspective

@jtmiclat
Copy link
Contributor Author

jtmiclat commented Jun 30, 2022

I wonder if it makes sense to have a function that takes in a geodataframe of multiple AOIs instead of manually iterating through each AOI and making separate geodataframes. I don't feel too strongly about this but might look "cleaner" from a user perspective

Moving this to the discussion!
#63

@jtmiclat jtmiclat marked this pull request as ready for review June 30, 2022 19:31
@jtmiclat jtmiclat requested a review from butchtm June 30, 2022 19:31
@butchtm butchtm merged commit c3d0266 into master Jul 1, 2022
@butchtm butchtm deleted the docs/document-grid-perfomance branch July 1, 2022 01:52
@tm-kah-alforja tm-kah-alforja changed the title Add documentaion for distanct AOIs for grid generation Add documentation for distant AOIs for grid generation Jul 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
3 participants