Implement HLG layer for P2P rechunking #8751
Conversation
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 29 files ± 0 29 suites ±0 11h 57m 8s ⏱️ + 5m 16s For more details on these failures, see this check. Results for commit 3703897. ± Comparison against base commit ffcb271. ♻️ This comment has been updated with latest results. |
|
|
||
| from dask.array.rechunk import old_to_new | ||
|
|
||
| _old_to_new = old_to_new(self.chunks_input, self.chunks) |
There was a problem hiding this comment.
I'll run some tests to figure out if this is bad, from what I understand, we should only cull once, i.e., this would get called twice in total.
| def cull( | ||
| self, keys: set[Key], all_keys: Collection[Key] | ||
| ) -> tuple[P2PRechunkLayer, dict]: |
There was a problem hiding this comment.
I don't love this... it feels like we're doing the same work twice. I know that the culling API forces us to generate this dict but it is still odd.
There was a problem hiding this comment.
Agreed, I'm still trying to figure out if there's a better way to avoid work here.
There was a problem hiding this comment.
I'm also worried about how this will look once we move to expressions where we no longer have the culling as such a concept. However, array expressions are in such an early stage of development that this hasn't been dealt with, yet, from what I can tell.
@phofl any insights by any chance (might also be easier to hop on a call)
Co-authored-by: Florian Jetter <fjetter@users.noreply.github.com>
|
An A/B test shows no significant impact on performance: |
Closes #8676
pre-commit run --all-files