Skip to content

fix: keep chunk-cached DataArray.data lazy (#2910) - #2912

Closed
RanaPriyansh wants to merge 1 commit into
Parcels-code:mainfrom
RanaPriyansh:fix/2910-lazy-chunk-cached-data
Closed

RanaPriyansh wants to merge 1 commit into
Parcels-code:mainfrom
RanaPriyansh:fix/2910-lazy-chunk-cached-data

Conversation

@RanaPriyansh

@RanaPriyansh RanaPriyansh commented Sep 28, 2026 •

Copy link
Copy Markdown

Summary

Accessing DataArray.data on a ChunkCachedArray field currently computes every backing Dask chunk. Return the backing Dask array from get_duck_array() so .data stays lazy. Vectorized selection continues to use the chunk cache.

Validation

  • Added a regression for lazy .data access and explicit .values and np.asarray materialization.
  • Verified vectorized selection values and repeat cache hits.
  • Ran the interpolation and chunk-cache test files: 42 passed, including one explicit chunk-cached XLinear regression.
  • Ruff lint, Ruff format, and focused mypy passed.

Fixes #2910

AI disclosure

  • This PR contains AI-generated content.
    • I have tested any AI-generated content in my PR.
    • I take responsibility for any AI-generated content in my PR.

Codex used.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that this test file is necessary. FWICT its pretty much just testing that dataarray.data is a Dask array (and testing Dask functionality).

I don't think its worth including in our test suite

Comment on lines 76 to +77
def get_duck_array(self):
return self.array.compute()
return self.array

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though there is the problem that users writing interpolators should never really be using da.data when using ChunkedArrays (since it falls back to Dask, which is less performant, or (before this PR) uses Numpy, which causes eager computation of results.

Implementing get_duck_array at all could result in users writing interpolators that have really bad performance.

I'm thinking maybe the solution is just to do a raise NotImplementedError here. This would mean that users can't inspect the data using a da.data, but I think thats acceptable (users won't be inspecting this anyway).

Thoughts @erikvansebille ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed that .data bypasses the chunk cache. I tested raising from get_duck_array at this head. It also makes .values and np.asarray(data_array) raise NotImplementedError; vectorized .isel(...).data still works. The API decision therefore includes whether explicit whole-array materialization should remain supported.

For test scope, the no-computation assertion fails on the original Parcels implementation because get_duck_array computes the chunks. The separate module can be reduced to a focused integration regression once the accessor contract is settled.

@VeckoTheGecko

Copy link
Copy Markdown
Contributor

superceded by #2916

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Avoid expensive casts to numpy arrays under the hood

2 participants