-
Notifications
You must be signed in to change notification settings - Fork 5
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
Support negative values #35
base: master
Are you sure you want to change the base?
Conversation
…t negative values
unrasterize/classes.py
Outdated
@@ -126,7 +126,7 @@ def _sort_pixels(band): | |||
) | |||
|
|||
@staticmethod | |||
def _reassign_pixel_values(band, pixels, raw_pixel_values=[]): | |||
def _reassign_pixel_values(threshold, band, pixels, raw_pixel_values=[]): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I have a slight preference for the argument order band, pixels, threshold, ...
@@ -126,7 +126,7 @@ def _sort_pixels(band): | |||
) | |||
|
|||
@staticmethod | |||
def _reassign_pixel_values(band, pixels, raw_pixel_values=[]): | |||
def _reassign_pixel_values(threshold, band, pixels, raw_pixel_values=[]): | |||
"""Adjust values of selected pixels so that their sum is preserved. | |||
|
|||
Parameters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also update the docstring with a description of the new function argument.
unrasterize/classes.py
Outdated
total_selected = np.sum(raw_pixel_values, dtype=np.float32) | ||
return [ | ||
out = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: This can be returned directly.
unrasterize/classes.py
Outdated
# Avoid underflow by ignoring negative values. | ||
total = np.sum(band[band > 0.0], dtype=np.float32) | ||
# Avoid underflow by ignoring values below the threshold. | ||
total = np.sum(band[band > threshold], dtype=np.float32) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced that passing threshold=self.threshold
works.
The example notebook uses an Unrasterizer(mask_width=10, threshold=0.2)
object. This function would then ignore all pixels with a population density < 0.2, so that they would not contribute to the total that is distributed among the selected pixels. In other words, the total sum of the selected pixels' values after applying this function would not equal the total sum across all pixels.
I'm not convinced that setting the new threshold parameter equal to That said, it's been a long time since I looked at this code in any depth. I assume from the code comments that I was running into mathematical underflow when the 0.0 value pixels were included in the summations. So perhaps it would make sense to include a parameter akin to Also note that the |
@tetraptych thanks for taking a look! I am tracking your comments and I think you're right, my solution is too simple and would not account for the actual total value above
Note that the actual sum of the value of the This is because pixels are not selected when below However, for windows where the band has no pixels that meet So, pixels below One solution to this is to add a line here that sets the value for any pixel below
This ensures that pixels whose value are below
This doesn't solve my issue around negatives but I think is a step in the right direction. I do now see the issue with sorting and agree using absolute values for the sort would work well. :sigh: this turned out to be far more nuanced than I expected! Thoughts? |
…s (see new Jupyter Notebook using CHIRPS rainfall anomaly data). Additionally, this adds an option where the user can specify whether they wish to sum or average across pixels.
@tetraptych I've pushed some additional updates to this PR. Negative values are now allowed using Let me know what you think! |
What's New
This PR allows
unrasterize
to process tiffs which may have negative values. This is accomplished by updating the_reassign_pixel_values
function to take in thethreshold
set by theUnrasterizer
class and using this as a cutoff to avoid underflow.This updates the more basic approach of just assuming a threshold of
0
for this function.This is useful for maps which may be measuring rates, for example population change. Some areas gain population and some lose it, but without this fix
unrasterize
will only select positive change pixels.