-
Notifications
You must be signed in to change notification settings - Fork 59
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
Ensure inputs of cosine of solar zenith angle are chunked when needed #1542
Conversation
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.
Very nice. I'm not sure if this would be appropriate, but I had the thought that cosine_of_solar_zenith_angle
and extraterrestrial_solar_radiation
could manage the chunking themselves by accepting an argument chunks
. Since they only accept those coordinate arrays, seems like something that will happen all the time. But, perhaps, we don't want to do these kinds of operations at this low level. Anyways, just a thought I'm sharing since I had nothing more to say, LGTM.
@coxipi I think that idea is better indeed! I changed to a |
@coxipi The previous CI run failed because of two bugs:
And with python 3.8, xarray can't go over 2023.1.0, so SPI and SPEI will fail for dask arrays in python 3.8. I suggest we do nothing about this as the plan is to drop 3.8 (and those older xarrays) in january. |
Pull Request Checklist:
number
) and pull request (:pull:number
) has been addedWhat kind of change does this PR introduce?
xc.core.utils._chunk_like
to chunk a list of inputs according to one chunk dictionary. It also circumvents [Bug]: cannot chunk a DataArray that originated as a coordinate pydata/xarray#6204 by recreating DataArrays that where obtained from dimension coordinates.uses_dask
so it can accept a list of objects._chunk_like
to ensure the inputs ofcosine_of_solar_zenith_angle
are chunked when needed, inmean_radiant_temperature
andpotential_evapotranspiration
.The effect of this is simply that the
cosine_of_solar_zenith_angle
will be performed on blocks of the same size as in the original data, even though its inputs (the dimension coordinate) did not carry that information. Before this PR, the calculation was done as a single block, of the same size as the full array.Does this PR introduce a breaking change?
No.
Other information:
Dask might warn something like
PerformanceWarning: Increasing number of chunks by factor of NN
. Where NN should be the number of chunks along thelat
dimension, if present. That exactly what we want, so it's ok.