GPU Refactor: Add array_type parameter and GPU extensions - #43
Open
hakkelt wants to merge 10 commits into
Open
Conversation
hakkelt
marked this pull request as draft
July 17, 2026 16:35
…t merge Adds AcceleratedDCTs test dependency, disables Aqua's persistent_tasks check, and carries forward the test coverage expansion, test refactors, and small dead-code/API cleanups developed on gpu-refactor after the GPU-support work was squash-merged upstream as 861af22.
Merge .github/agents, .github/skills, and .github/instructions into one flat AGENTS.md and add a CLAUDE.md symlink, removing the duplication across the router file and its six referenced documents.
Benchmark Results (Julia v1.12.6)No significant performance or memory regressions detected. Time benchmarks
Memory benchmarks
|
It requires Julia >= 1.11 and is already added dynamically at runtime for that case in test/utils.jl; listing it statically forced dependency resolution to fail on the Julia 1.10 LTS CI job.
JLArray satisfies StridedArray/DenseArray, so AbstractFFTs.plan_fft
dispatches to FFTW's raw-pointer CPU plan instead of a GPU-safe path,
and JLArrays intentionally throws on the resulting unsafe_convert to
Ptr ("Illegal conversion of a JLArray to a Ptr"). This broke the
DFT/RDFT/IRDFT, Conv, Xcorr, Filt, and MIMOFilt GPU testitems, which
were missing the include_jlarrays = false guard that the neighboring
DCT/IDCT (GPU) testitem already uses for the same reason.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #43 +/- ##
==========================================
+ Coverage 87.13% 90.13% +2.99%
==========================================
Files 51 51
Lines 3716 3679 -37
==========================================
+ Hits 3238 3316 +78
+ Misses 478 363 -115 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…ratorWrapper These types had no direct test of the storage_type/threaded deep-copy path added by copy_operator's refactor, so the codecov patch-coverage gate flagged their _copy_operator_impl methods as untested. Also adds a direct test of copy_operator's fast-path/slow-path dispatch and the default (deepcopy) fallback. Each new test forces the deep-copy path with an explicit non-default kwarg, since the fast path (thread-safe operator, no kwargs) just returns the same operator unchanged. Manually verified against the package (TestItemRunner/GPUEnv itself hit an unrelated local Pkg.jl manifest bug, JuliaLang/Pkg.jl#4086, on this sandbox).
Julia's === for immutable structs can compare egal by field content rather than allocation identity, and this differs across Julia versions: the Variation test's !== assertion passed on 1.12 locally but failed on the 1.10 LTS CI job because the copied Variation ended up field-identical to the original. Removed the same fragile pattern from the other new copy_operator tests (Sum, BroadCast, OperatorWrapper, the MatrixOp fast/slow-path test) and kept only the functional (mul!) equivalence and isa checks, matching the already-stable HadamardProd test's style. DiagOp's op3.d !== op.d assertion is unaffected since it compares two Array objects directly, not immutable operator structs.
…cing_expr Both were still flagged as uncovered by codecov's patch-coverage gate despite being exercised by existing tests, so add more direct tests: an explicit threaded=true call for HadamardProd's _copy_operator_impl, and a 3-element HCAT to unambiguously exercise the get_slicing_expr loop body across multiple iterations.
Root AGENTS.md carried directory-layout and generic Julia perf advice a session can already derive, plus two large sections that only matter in specific contexts. Move GPU extension conventions to ext/GpuExt/AGENTS.md (loads only when working in that directory) and the long-running test/coverage/benchmark workflow to a skill, keeping CLAUDE.md as a symlink to AGENTS.md per repo convention. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JHYpJyvm5XwhtMXLE1N8i9
Add copy_operator tests for HCAT and HadamardProdJac, and a test for the generic domain/codomain_array_type fallback (_storage_type_for_elem with a Tuple), closing the coverage gaps flagged in the PR kul-optec#43 Codecov patch report. Writing the HadamardProdJac test surfaced a real bug: its _copy_operator_impl used _convert_buffer (shape-only, no value copy) on bufA/bufB, which hold the Jacobian's linearization point (A*x, B*x) carried over from the originating HadamardProd rather than pure scratch. Copying wiped that state, producing garbage adjoints. Fixed by copying bufA/bufB with copy/copyto! (preserving values, still honoring storage_type conversion), leaving bufD on the fast path since it's pure scratch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JHYpJyvm5XwhtMXLE1N8i9
hakkelt
marked this pull request as ready for review
August 14, 2026 08:35
Collaborator
Author
|
@lostella this PR is ready to review. It contains no significant changes, just minor enhancements and fixes, and a lot of new tests. |
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
1 participant
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.
This PR introduces a comprehensive GPU refactoring, plus test-coverage, CI-stability, and agent-guidance follow-ups.
GPU support
array_typeparameter to support multiple backends.GPUEnvfor backend-agnostic testing.StridedArrayconformance routesAbstractFFTs.plan_fftinto FFTW's raw-pointer CPU path, which JLArray intentionally rejects.Test coverage & CI stability
copy_operatorcoverage forSum,DiagOp,Variation,BroadCast,OperatorWrapper,HadamardProd, andHCAT'sget_slicing_expr, closing codecov patch-coverage gaps.!==struct-identity assertions fromcopy_operatortests (Julia's immutable-struct===semantics differ across versions and broke the 1.10 LTS job).AcceleratedDCTsfromtest/Project.toml(Julia ≥1.11-only; added dynamically at runtime instead, since a static listing broke 1.10 LTS dependency resolution).persistent_taskscheck.Agent guidance
.github/agents,.github/skills, and.github/instructionsinto a singleAGENTS.md, with aCLAUDE.mdsymlink.ext/GpuExt/AGENTS.mdand the long-running test/coverage/benchmark workflow into a skill, so both load only when relevant instead of always.