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

Force use of stated nodata value and float32 dtype in pollination #1636

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

phargogh
Copy link
Member

This PR updates all calls to pygeoprocessing.raster_map to use the pollination-defined nodata constant value, and also updates the dtype to force the output dtype to float32, resulting in smaller filesizes.

Some parts of the model expected a specific nodata value, which meant that raster_map's calculation of a reasonable nodata value was resulting in near-inf outputs in some rasters.

Likely because of the use of convolutions in the pollination model (which create float64 rasters), most of the model outputs were also being created as float64 rasters, which probably isn't strictly necessary. This change just reverts back to float32 precision, which saves some disk space in the output workspace.

Fixes #1635

Checklist

  • Updated HISTORY.rst and link to any relevant issue (if these changes are user-facing)
    - [ ] Updated the user's guide (if needed)
    - [ ] Tested the Workbench UI (if relevant)

Copy link
Member

@emlys emlys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@phargogh the pollination changes make sense to me! In the parts of the model that expect that specific nodata value, I wonder if it might also make sense to instead read the nodata value from the raster properties? Rather than relying on the previously-set value.

There's also some changes in here in convert-requirements-to-conda-yml.py and urban_nature_access.py that may be unrelated?

@phargogh
Copy link
Member Author

phargogh commented Oct 8, 2024

OK! I just pulled in the latest changes from main and removed the changes to the conda conversion script that I had made trying to debug build failures. The UNA change appears to have happened because I must have based this branch off of the branch for #1631, so once it's all looking good, let's merge this after that other PR.

@phargogh phargogh requested a review from emlys October 8, 2024 20:34
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 this pull request may close these issues.

Pollination farm_pollinators.tif has inf values when run with the sample data
2 participants