Array-API-native block_reduce, block_average and block_replicate off numpy - #1009
Open
mwcraig wants to merge 5 commits into
Open
Array-API-native block_reduce, block_average and block_replicate off numpy#1009mwcraig wants to merge 5 commits into
mwcraig wants to merge 5 commits into
Conversation
astropy.nddata's block functions start with numpy.asanyarray, so on a non-numpy array library they hand back a numpy array (dask, jax) or fail outright when the data are on a device numpy cannot reach (array-api-strict, cupy). ccdproc/_blocks.py does the same work using only array API operations -- reshape, permute_dims, repeat and slicing -- so the result stays in the caller's namespace and on the caller's device. block_size validation is pure Python and reproduces astropy's three checks, in astropy's order, with astropy's messages. Both functions are decorated with astropy.nddata.support_nddata, as astropy's own are, so a CCDData argument is unpacked and the "following attributes were set ... will be ignored" warning is emitted identically. The one deliberate divergence is dtype: block_replicate(conserve_sum=True) promotes integer and boolean input to the namespace's default real floating dtype before dividing, because array-api-strict rejects integer true division rather than promoting. numpy returns float64 there anyway. _nanfuncs._fill_doc gained a positional-only template argument so the new module can reuse the docstring templating with its own parameter block. Part of astropy#971. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN
ccdproc.core.block_reduce/block_average/block_replicate now resolve the array namespace first and keep calling astropy.nddata only when it is numpy; every other namespace gets ccdproc._blocks. The numpy branch is the code that was there before, so numpy results are unchanged, and the CCDData rebuild and the ignored-attribute warning are untouched. block_replicate gained the xp= argument the other two already had. With the escape gone, drop the three block_* lines from the array-escape baseline and the three array-api-strict xfail markers on the block tests. Those tests then failed one step later on xp.zeros(..., dtype=bool), which array-api-strict rejects; they now ask for xp.bool, as the rest of the file does. Part of astropy#971. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN
Part of astropy#971. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN
numpy's mean promotes an integer array on its own, and jax and dask follow it, but array-api-strict refuses a non-floating mean outright, so an integer image averaged fine on three backends and raised on the fourth. ccdproc._blocks.block_average now promotes integer and boolean input to the namespace's default real floating dtype before reducing with xp.mean, the same treatment block_replicate already had, and core.block_average dispatches to it. The numpy path is unchanged and still goes straight to astropy.nddata. block_average has no astropy counterpart, so it lives here as ccdproc's own thin wrapper; only block_reduce and block_replicate are candidates for the upstream lift. The docstring parameter template gained an empty-``extra`` form, since block_average has no function-specific parameter between block_size and xp. Part of astropy#971. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1009 +/- ##
==========================================
+ Coverage 97.97% 98.05% +0.07%
==========================================
Files 9 10 +1
Lines 1927 2005 +78
==========================================
+ Hits 1888 1966 +78
Misses 39 39
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Codecov flagged two untested branches in astropy#1009: the early return in _block_namespace when the caller passes xp, and the int() failure branch in _block_size that turns a NaN or infinite block size into astropy's "must be integers" error. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ccdproc.core.block_reduce,block_averageandblock_replicateare thinwrappers around
astropy.nddata, which starts by callingnumpy.asanyarrayon its input. On a non-numpy array library that silently hands back a numpy
array (dask, jax) or fails outright when the data live on a device numpy
cannot reach (array-api-strict, and by extension cupy on a GPU). These were
three of the remaining array-API escapes in the escape baseline.
This adds
ccdproc/_blocks.py, which does the same work using only array APIoperations (
reshape,permute_dims,repeatand slicing), and dispatchesto it for every namespace that is not numpy.
This is deliberately a stopgap: it exists so ccdproc's array-API work does not
have to wait for an astropy release.
_blocks.block_reduceand_blocks.block_replicate, minus the ccdproc dispatch, are the intendedstarting point for astropy/astropy#15073; once astropy's own
blocks.pyisarray-API aware and that version is ccdproc's floor, this module and the
dispatch both go away. (
block_averagehas no astropy counterpart -- it isccdproc's own thin wrapper -- so only the other two are candidates for the
lift.)
Dispatch policy
The numpy path is unchanged:
if array_api_compat.is_numpy_namespace(xp)thefunctions call
astropy.nddataexactly as before, with the same arguments.Everything else goes to
_blocks. The CCDData rebuild and the"following attributes were set ... will be ignored" warning behaviour are
untouched -- the native helpers are decorated with
astropy.nddata.support_nddata, as astropy's own are, so aCCDDataargument is unpacked and warned about identically on every backend.
block_sizevalidation is done in pure Python and reproduces astropy's threechecks in astropy's order with astropy's messages verbatim; the tests assert
against astropy's live output rather than hard-coded strings so the two cannot
drift apart.
Behaviour differences
Results are identical to astropy's except for two dtype promotions, both of
integer/boolean input to the namespace's default real floating dtype:
block_replicate(..., conserve_sum=True)promotes before dividing, becausearray-api-strict rejects integer true division rather than promoting.
block_averagepromotes before averaging, because array-api-strict rejectsa non-floating
mean. numpy promotes on its own here, and jax and daskfollow it, so without this an integer image averaged fine on three backends
and raised on the fourth.
numpy returns
float64in both cases anyway, so these differ only for alibrary whose default real dtype is not
float64. Documented indocs/array_api.rstandCHANGES.rst.Verified backends
Run with the dev environment directly,
CCDPROC_ARRAY_LIBRARY=<lib>(plusJAX_ENABLE_X64=1for jax); array-api-strict runs on its non-defaultdevice1, which is wherenp.asarrayraises.pytest ccdproc/tests/test_blocks.py ccdproc/tests/test_ccdproc.py:Full suite, this branch vs. the merge base:
The deltas are fully accounted for: the 70 new tests in
test_blocks.py, plusthe three formerly-xfailed array-api-strict tests now passing.
The three
block_*lines are removed fromccdproc/tests/array_escape_baseline.txt. Verified with a full dask run underCCDPROC_LOG_ARRAY_ESCAPES=1 CCDPROC_ENFORCE_ESCAPE_BASELINE=1: no escapesoutside the baseline, no
block_*sites in the escape log, and temporarilyrestoring the three lines makes the ratchet report exactly those three as
"not hit this run" and nothing else.
Things worth a reviewer's eye
_nanfuncs._fill_docgained a positional-onlytemplateargument so_blocks.pycan reuse the docstring templating with its own parameterblock. Backwards compatible; no
_nanfuncsdocstring changed.core._block_namespaceresolves the namespace asccd.data if isinstance(ccd, CCDData) else ccd, matchingsigma_func'sconvention. The old code used
ccd.dataunconditionally, which crashed fora bare array (
np.ndarray.datais a memoryview) -- bare arrays now work.The narrowing is that a plain list passed with an explicit
funcused toreach astropy and now raises
TypeErrorfromarray_namespace; nothing inthe test suite or docs does that.
xp.isdtype(dtype, ("integral", "bool"))rather than"not real floating", so complex input is not silently truncated to real.
tests failed one step later on their own
xp.zeros((4, 4), dtype=bool),which array-api-strict rejects; they now ask for
dtype=xp.bool, as therest of that file already does. The
ccd._mask = ...TODO is untouched.test_blocks.pyships with it andincludes two tests of the
coredispatch that need the second commit onnon-numpy backends. Squashing the first two commits would fix that if you
care about per-commit bisection on the backend jobs.
Part of #971.
🤖 Generated with Claude Code
https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN