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.

Sign up for GitHub

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

Jump to bottom

Cosine of solar zenith angle takes way too much memory for long datasets #1536

Closed
aulemahal opened this issue Nov 23, 2023 · 0 comments · Fixed by #1542
Closed

Cosine of solar zenith angle takes way too much memory for long datasets #1536

aulemahal opened this issue Nov 23, 2023 · 0 comments · Fixed by #1542
Assignees
@aulemahal
Labels
bug Something isn't working enhancement New feature or request
Milestone
v0.47.0

Comments

@aulemahal
Copy link
Collaborator

aulemahal commented Nov 23, 2023 •
edited
Loading

The computation inside cosine_of_solar_zenith_angle creates an array with temporal and spatial dimensions because it multiplies the sins of declination and lat. However, declination was created from the time coordinate which doesn't carry chunking information, as it is a dimension1. Similarly, there are many cases where lat is a dimension without chunking information.

Result : the created array is enormous even though the initial dataset could have been appropriately chunked.

Solutions:

  1. Add a chunks argument to rechunk the arrays before multiplying them.
  2. Chunking the inputs before sending them to the function. That might necessitate renaming the coordinates, to ensure the arrays are not "dimensions" anymore

This affects the potential evapotranspiration indicator for example, with no way for the user to change the behaviour.

Footnotes

  1. By "dimension" I mean a 1D array that is a coordinate and has the same name as its dimension. Xarray forbids chunking on those and will automatically load them. ↩
@aulemahal aulemahal added bug Something isn't working enhancement New feature or request labels Nov 23, 2023
@aulemahal aulemahal added this to the v0.47.0 milestone Nov 23, 2023
@aulemahal aulemahal self-assigned this Nov 23, 2023
@aulemahal aulemahal mentioned this issue Nov 29, 2023
Merged
5 tasks
aulemahal added a commit that referenced this issue Nov 30, 2023
@aulemahal
Ensure inputs of cosine of solar zenith angle are chunked when needed (…
cf4bf5a 
…#1542)

<!--Please ensure the PR fulfills the following requirements! -->
<!-- If this is your first PR, make sure to add your details to the
AUTHORS.rst! -->
### Pull Request Checklist:
- [x] This PR addresses an already opened issue (for bug fixes /
features)
    - This PR fixes #1536
- [x] Tests for the changes have been added (for bug fixes / features)
- [ ] (If applicable) Documentation has been added / updated (for bug
fixes / features)
- [x] CHANGES.rst has been updated (with summary of main changes)
- [x] Link to issue (:issue:`number`) and pull request (:pull:`number`)
has been added

### What kind of change does this PR introduce?

* New function `xc.core.utils._chunk_like` to chunk a list of inputs
according to one chunk dictionary. It also circumvents
pydata/xarray#6204 by recreating DataArrays
that where obtained from dimension coordinates.
* Generalization of `uses_dask` so it can accept a list of objects.
* Usage of `_chunk_like` to ensure the inputs of
`cosine_of_solar_zenith_angle` are chunked when needed, in
`mean_radiant_temperature` and `potential_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
the `lat` dimension, if present. That exactly what we want, so it's ok.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aulemahal aulemahal

Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Milestone
v0.47.0
Development

Successfully merging a pull request may close this issue.

Ensure inputs of cosine of solar zenith angle are chunked when needed Ouranosinc/xclim
1 participant
@aulemahal