Add conditional to set _mv variable to np.uint64 data type #47
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…f large arrays. Ref Deltares#46
…ype, and _mv assertion
| self._mv = core._mv | ||
| if idxs_ds.dtype == np.uint32: | ||
| self._mv = np.uint32(self._mv) | ||
| if idxs_ds.dtype == np.uint64: |
There was a problem hiding this comment.
Just appears to be a dtype limitation?
There was a problem hiding this comment.
@fernando-aristizabal not exactly. The dtype assigned to self._mv needs to be the same as the dtype for idxs.ds. Those two dtypes need to line up, otherwise there were errors in numba code. In the downstream code, incorrect aliasing was occurring based on _mv's dtype. This was causing a mis-match in values between _mv and items in the idxs.ds array, which ultimately led to the IndexErrors being thrown.
In order to avoid the IndexErrors, for the large array indices case, where the idxs.ds.dtype is set to uint64, the dtype for _mv needs to be set to uint64 as well.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Allow the processing of large rasters, shaped at (63708, 79780), for
Flwdirobject.Issue addressed
Closes #46
Explanation
As described in issue #46, when large rasters are used, an IndexError was thrown from underlying numba code.
The upstream_count function was the cause of the Indexing Error, due to an incorrect value set in the
_mvvariable when using anp.uint64data type.Checklist
mainAdditional Notes (optional)
The setting of the np.uint64 dtype in pyflwdir.py was not accounted for, and caused non-correct values in the
_mvvariable, when assigned in core.py. This was causing the C pointer references to not line up with the signed integer type that was being used in python. See numpy docs fornp.intp()The downstream effects