-
Notifications
You must be signed in to change notification settings - Fork 40
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
np.isnan triggers eager computation #218
Comments
But maybe not; xskilscore's is nested under apply_ufunc |
I don't think it does but hard to dissect the steps under the hood. Probably easiest is to explore in the binder: https://mybinder.org/v2/gh/raybellwaves/xskillscore-tutorial/master?urlpath=lab |
You could also just make a |
dont we have tests that check this? xskillscore/xskillscore/tests/test_deterministic.py Lines 212 to 247 in 5252261
seems not. test_deterministic.py needs to be refactored to make it more readable to see whats missing. also by trying to have as many fixtures as possible, i.e. adding xskillscore/xskillscore/tests/test_deterministic.py Lines 133 to 138 in 5252261
|
That doesn't actually test whether it triggers eager computation or not; only whether it works (even though it triggered an eager computation somewhere in between). Could add an overloading test that reflects Riley's comment: "You could also just make a dask array that's much larger than your system's memory and see if it breaks when chunking and sending it through np.isnan. My guess is that apply_ufunc avoids eagerly evaluating it." |
xarray has some tests that check whether they trigger any computation. Also in the PR you mention they seemed to have found a nice solution which could also be our solution. |
What Aaron is referencing:
|
Ha it does trigger a computation!
|
After looking at the code and trying a few things, I think we may need to move call _match_nans in the xarray level (use map blocks like the PR is doing) instead of calling it from np_deterministic. I tried the following, but the if statement still triggers it.
|
how about their |
Yes this needs to be done before xr applyufunc
…On Wed, Nov 4, 2020, 1:52 AM Aaron Spring ***@***.***> wrote:
how about their .map_blocks solution?
https://github.com/pydata/xarray/pull/4559/files#
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#218 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADU7FFR7NVK7UASVMO6LQOLSOEB5RANCNFSM4TA36MYA>
.
|
Actually, the above doesn't trigger computation (I think I might have accidentally called compute() above), it was the weights that trigger computation:
|
so setting the |
not quite. the if is evaluated eagerly. |
Please ping me if you need any thoughts specifically from me on this. T minus one week on dissertation writing and then will be taking a short break after that. Just trying to pop in and address anything that folks are waiting on me for. |
Nah nothing from me, I still haven't tested because I'm working on a personal project at the moment which is absorbing all my time outside of work~ |
Closed via #224 |
I was reading about this:
pydata/xarray#4541
And I think xskillscore's also trigger an eager computation:
https://github.com/xarray-contrib/xskillscore/blob/master/xskillscore/core/np_deterministic.py#L23
The text was updated successfully, but these errors were encountered: