Skip to content

Make the host round trip explicit in wcs_project, subtract_overscan and cosmicray_lacosmic - #1011

Open
mwcraig wants to merge 13 commits into
astropy:mainfrom
mwcraig:host-copy-policy
Open

Make the host round trip explicit in wcs_project, subtract_overscan and cosmicray_lacosmic#1011
mwcraig wants to merge 13 commits into
astropy:mainfrom
mwcraig:host-copy-policy

Conversation

@mwcraig

@mwcraig mwcraig commented Sep 7, 2026

Copy link
Copy Markdown
Member

Part of #971. Closes #930, closes #933, closes #935. Independent of #1009 and #1010 except for one expected conflict in ccdproc/tests/test_cosmicray.py (see below).

What changed and why

Three ccdproc operations depend on a library that is NumPy-only: wcs_project (reproject), subtract_overscan with a model (astropy.modeling fitting) and cosmicray_lacosmic (astroscrappy). Until now they each let their input leak to NumPy implicitly and handed a NumPy array back to a caller who had passed jax, dask or a device array. This PR makes that host round trip explicit and symmetric:

  • A new ccdproc.HostCopyWarning (subclass of AstropyUserWarning, exported from ccdproc) is emitted once per call for non-NumPy input, attributed to the caller's own line (the stacklevel accounts for the log_to_metadata / deprecated_renamed_argument decorators on all three functions). NumPy input is neither copied nor warned about. The warning is filtered out in the test configuration.
  • Two helpers next to _to_numpy in core.py: _from_numpy(array, like=..., xp=...) converts a result back to the namespace and device of the input, and _warn_host_copy(name, xp, stacklevel=...) issues the warning.
  • wcs_project: data and mask are copied to the host explicitly, the reprojected data and mask are converted back, the mask is combined in the namespace via _mask, and the pixel-area ratio is a Python float (astropy returns a NumPy scalar, which array-api-strict refuses as an operand). target_shape is now coerced to a tuple of Python ints so any two-integer sequence or array is accepted, including an array from ccd's namespace; reproject needs len() of it and array-api-strict arrays have none.
  • subtract_overscan with a model: the fit and its evaluation run in NumPy on the host and the result is converted back.
  • cosmicray_lacosmic, both the CCDData and the bare-array branch: data, mask and any array-valued inbkg/invar go to the host; cleanarr and crmask come back in the caller's namespace and device. The bare-array branch now returns caller-namespace arrays too.
  • _astroscrappy_gain_apply_helper multiplies by float(gain.value) instead of the NumPy float64 gain.value. array-api-strict rejects a NumPy scalar as an operand (TypeError: Expected Array or Python scalar); gain_correct already did this coercion at its own call site. This is what the two "gain and mask" strict markers were really recording.

Tests and markers

  • New ccdproc/tests/test_hostcopy.py (16 tests per backend) pins the contract: the warning is emitted exactly once, attributed to the caller, only for non-NumPy input; every returned array is in the input's namespace and on the input's device; NumPy input is silent and uncopied; and a scalar Quantity gain keeps namespace and device on array-api-strict's non-default device (the Array API: units/Quantity handling with non-numpy arrays #936 audit, see below).
  • The array-api-strict backend_xfail markers these three functions had on test_ccdproc.py and test_cosmicray.py are removed; on strict, 31 previously-xfailed test instances now pass. The subtract_overscan line is deleted from array_escape_baseline.txt; the escape ratchet (CCDPROC_LOG_ARRAY_ESCAPES=1 CCDPROC_ENFORCE_ESCAPE_BASELINE=1, full dask run) reports no escapes outside the baseline and no stale entries. All three sites' copies are attributed to core.py _to_numpy, so no new caller-attributed lines were needed.
  • Tests that ran on strict for the first time needed NumPy-isms removed: .sum() calls, assert_allclose on device arrays (now via _to_numpy, the pattern test_combiner.py uses), Python-int slice bounds, and add_cosmicrays copying through a host round trip because a host copy of a jax array is read-only. The no_cosmics stand-in for detect_cosmics now returns NumPy arrays like the real one, so it exercises the conversion back.
  • test_wcs_project_onto_scale_wcs's input mask is a bool array rather than a float one. reproject casts to float internally either way; for the nearest-neighbour order the test uses the outputs are bit-identical.

Results

Full pytest ccdproc on each backend (JAX_ENABLE_X64=1 for jax), compared with main at cfdbdd4:

backend main this branch
numpy 964 passed, 35 skipped 977 passed, 39 skipped
jax 963 passed, 36 skipped 977 passed, 39 skipped
dask 957 passed, 42 skipped 971 passed, 45 skipped
array-api-strict 917 passed, 36 skipped, 46 xfailed 962 passed, 39 skipped, 15 xfailed

No test regressed on any backend. Every backend gains exactly 17 tests: the 16 in test_hostcopy.py (their pass/skip split differs per backend because the warning and no-warning tests are complementary) and the new target_shape test. On strict the 31 previously-xfailed instances all pass. Skip counts are higher than in CI because this machine lacks bottleneck and is not Linux.

#936

Audited as part of this: every Quantity ccdproc constructs on the gain/readnoise path wraps a Python scalar, so none of them can pull data out of its namespace or off its device. What did bite was the gain.value NumPy scalar above, now fixed and pinned by test_scalar_quantity_gain_keeps_namespace_and_device. An array-valued gain or readnoise would need upstream astropy work (data << unit in the arithmetic mixin is not array-API aware) and is the remaining test_unit_mismatch_behaves_as_expected xfail, tracked separately. A closing comment for #936 will follow separately rather than via this PR.

Expected conflict with #1010

add_cosmicrays in ccdproc/tests/test_cosmicray.py is shared with the cosmicray_median tests #1010 touches, and both PRs remove markers in that file. Whichever lands second rebases; the conflicts are adjacent-line.

Not verified

  • CuPy: not installed, so untested, as usual for this repo.
  • The docs build was not run locally (sphinx-astropy is not installed here); CI will check the new docs/array_api.rst section and the three docstring Notes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN

mwcraig and others added 12 commits September 7, 2026 16:02
@mwcraig @claude
Add HostCopyWarning and the _from_numpy/_warn_host_copy helpers
ef7064d 
The three CPU-only operations (wcs_project, subtract_overscan with a
model, cosmicray_lacosmic) depend on numpy-only libraries. They will make
their host round trip explicit: _to_numpy on the way in, _from_numpy on
the way back, and one HostCopyWarning per call announcing that the copy
happened.

_from_numpy is the inverse of the existing _to_numpy: it restores both the
array namespace and the device of the caller's data, and passes None
through so an absent mask needs no special case. _warn_host_copy is a
no-op on numpy, so the numpy path neither converts nor warns.

HostCopyWarning is a subclass of AstropyUserWarning and is exported
through core.__all__, so users can silence it with
warnings.filterwarnings("ignore", category=ccdproc.HostCopyWarning). The
test suite runs with filterwarnings = error, so the same filter is added
to pyproject.toml: the warning contract is pinned once, in a dedicated
test module, rather than by wrapping every affected test in pytest.warns.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN
@mwcraig @claude
Make wcs_project's host round trip explicit (closes astropy#930)
a6d909c 
reproject is numpy-only. wcs_project used to hand it whatever array it was
given, let it convert silently, and then build the returned CCDData out of
the numpy result -- so a dask, jax or CuPy caller got a numpy-backed image
back, and array-api-strict on a non-default device could not run the
function at all.

The data and the mask now go to the host through _to_numpy and come back
through _from_numpy, in the caller's namespace and on its device, and the
call warns once with HostCopyWarning. Two numpy-isms in the surrounding
code go with it: the "nothing masked" test uses xp.any rather than the
.any() method, and area_ratio is made a Python float because astropy hands
back a numpy scalar that strict namespaces reject as an operand. The mask
is assigned through _mask, as ccd_process does, because astropy's mask
setter coerces with np.asarray.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN
@mwcraig @claude
Make subtract_overscan's model fit run on the host explicitly (closes a…
78efad4 
…stropy#933)

astropy.modeling is numpy-only. The model path used to hand the fitter an
xp array and an xp yarr, let astropy convert both silently, and then
restore only the namespace of the result with xp.asarray -- losing the
device, so array-api-strict on a non-default device failed.

The overscan vector now goes to the host through _to_numpy, yarr is built
with numpy, the fit and its evaluation happen entirely in numpy, and the
fitted overscan comes back through _from_numpy in the caller's namespace
and on its device. The call warns once with HostCopyWarning. The median
and mean paths are untouched: they are already native.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN
@mwcraig @claude
Make cosmicray_lacosmic's host round trip explicit
95bcd41 
astroscrappy is numpy-only. Both branches used to hand it the caller's
arrays and let it convert them silently; the CCDData branch then restored
only the namespace of the cleaned data (losing the device) and assigned
the numpy cosmic-ray mask straight onto the result, and the bare-array
branch returned astroscrappy's numpy arrays unchanged.

The data, the input mask and any array-valued inbkg/invar now go to the
host through _to_numpy, and the cleaned data and cosmic-ray mask come back
through _from_numpy before any arithmetic, so the offset and gain
arithmetic that follows runs in the caller's namespace. The mask is
combined with xp.logical_or and assigned through _mask, since the mask
setters do not preserve the device. The bare-array branch returns both
arrays in the caller's namespace and on its device. Each call warns once
with HostCopyWarning.

gain.value is a numpy scalar, which array-api-strict refuses as an
operand, so it is made a Python float before it meets an array -- the same
thing gain_correct already does. _astroscrappy_gain_apply_helper needed no
change: it only multiplies and divides.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN
@mwcraig @claude
Drop the strict markers the explicit host round trip made stale
0e113a2 
Removes the 31 array-api-strict expected failures whose reason was that
wcs_project, subtract_overscan(model=) or cosmicray_lacosmic "requires
numpy and fails on a non-default device". They pass now.

That includes the two markers whose reason was instead
"cosmicray_lacosmic's gain and mask paths do not support array-api-strict's
strict scalar and dtype rules": the operation they were hiding was
``cleaned_data * gain.value`` with a numpy scalar gain, which the previous
commit fixed, so they are gone too rather than being restored with a
sharper reason. test_unit_mismatch_behaves_as_expected keeps its marker;
its cause is astropy's arithmetic mixin, not the host copy.

With the markers gone the tests actually run on array-api-strict for the
first time, which turned up numpy-isms in the test code itself: .sum() on
arrays, numpy.testing.assert_allclose on foreign arrays, np.array on an
array that lives on a non-default device, ``dtype=int``, an array used as
a slice bound, and an array target_shape handed to reproject. Those are
fixed here. The stand-in for detect_cosmics now returns numpy arrays, as
the real one does, so it exercises the conversion back. add_cosmicrays
copies through _to_numpy and puts the result back on the device it came
from, taking a writable copy because the host copy of a JAX array is
read-only. The four wcs_project tests no longer need their "hack for
numpy-specific check in astropy.wcs": wcs_project does the conversion now.

The subtract_overscan entry leaves the escape baseline: its numpy escape
is now attributed to _to_numpy, the deliberate boundary, which stays.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN
@mwcraig @claude
Pin the host-copy warning and round-trip contract
e8504d4 
New test module for the policy the previous commits implement, since the
suite-wide ignore filter means no other test can see the warning.

It pins that HostCopyWarning is reachable as ccdproc.HostCopyWarning and is
an AstropyUserWarning; that _from_numpy restores namespace, device and
dtype and passes None through; that each of the three CPU-only functions
emits exactly one warning naming itself on a non-numpy backend and none at
all on numpy (recorded with simplefilter("always"), so a regression that
started converting numpy input would be caught); that the data and mask
each returns are in the caller's namespace and on its device, including
the bare-array branch of cosmicray_lacosmic; and that
combine(output_file=...) copies to the host without warning, as the policy
says it should.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN
@mwcraig @claude
Document the CPU-only host round trip
540f713 
Adds a "Which operations run on the CPU?" section to docs/array_api.rst
stating the policy: an operation built on a numpy-only library copies its
input to the host, runs, and copies every array it returns back to the
caller's namespace and device; it warns once per call site with
HostCopyWarning; numpy never copies and never warns;
combine(output_file=...) copies without warning because nothing comes
back; and there is no knob to disable the conversion and no native
reimplementation planned.

The docstrings of the three functions each gain a Notes paragraph saying
the same thing in one place, and CHANGES.rst gets the new-feature entry
(astropy#930, astropy#933, astropy#935) plus a bug-fix entry for wcs_project returning numpy
for non-numpy input (astropy#930).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN
@mwcraig @claude
Pin that a scalar Quantity gain keeps namespace and device (closes as…
506270b 
…tropy#936)

astropy#936 asked whether the Quantity arithmetic in ccdproc drags data out of
its array namespace. An audit of the four Quantity constructions in
core.py -- the gain_correct default, the lacosmic gain and readnoise
defaults, and the gain.value * u.one special case -- found that all of
them wrap Python scalars, so there is no local bug to fix. This test
records that: gain_correct and cosmicray_lacosmic, the two public
functions that build arrays out of gain.value, keep the data in its
namespace and on its device when given a scalar Quantity gain. On
array-api-strict the test runs on a non-default device, so a lost device
fails it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN
@mwcraig @claude
Say in _astroscrappy_gain_apply_helper's docstring that it is namespa…
6025a3e 
…ce-agnostic

The helper now receives the cleaned data in the caller's array namespace,
not as a numpy array, and its gain argument must be a Python float rather
than the numpy scalar gain.value returns.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN
@mwcraig @claude
Accept any two-integer sequence or array as wcs_project's target_shape
a52d1c1 
reproject needs len() of the shape and elementwise comparison, which
an array-api-strict array does not support, so coerce target_shape to
a tuple of Python ints up front. A shape is metadata, not data, and
the docstring already promised a list-like.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN
@mwcraig @claude
Point the changelog entries at the pull request
e48902c 
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN
@mwcraig @claude
Filter HostCopyWarning in the test config by message, not by class
70f2293 
Naming ccdproc.HostCopyWarning in pytest's filterwarnings makes pytest
import ccdproc while it parses pyproject.toml. Under the CI invocation,
pytest --pyargs ccdproc, that loads the installed package from
site-packages before the source-tree conftest, which then fails with an
ImportPathMismatchError. Reproduced with tox locally; fixed by matching
the warning's message instead.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN
@codecov

codecov Bot commented Sep 7, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.05%. Comparing base (9d18599) to head (07ac1ac).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1011      +/-   ##
==========================================
+ Coverage   97.97%   98.05%   +0.07%     
==========================================
  Files           9        9              
  Lines        1927     1953      +26     
==========================================
+ Hits         1888     1915      +27     
+ Misses         39       38       -1
Flag Coverage Δ
dask 97.28% <100.00%> (+0.08%) ⬆️
jax 97.48% <100.00%> (+0.13%) ⬆️
numpy 97.90% <97.43%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mwcraig @claude
Pin that cosmicray_lacosmic merges an existing mask after the round trip
07ac1ac 
The branch that ORs a CCDData's existing mask with the cosmic-ray mask
was never exercised on any backend, on main or here, and codecov flagged
it once this pull request rewrote it. The new test plants one cosmic
ray, masks an unrelated pixel, and checks that the result is the union
of the two in the caller's namespace and on its device.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN
@mwcraig mwcraig mentioned this pull request Sep 7, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

1 participant

@mwcraig