Skip to content

fix: eval_grad_tree_array works with SubArray inputs#566

Open
MilesCranmerBot wants to merge 2 commits intoastroautomata:masterfrom
MilesCranmerBot:fix/eval-grad-view-subarray
Open

fix: eval_grad_tree_array works with SubArray inputs#566
MilesCranmerBot wants to merge 2 commits intoastroautomata:masterfrom
MilesCranmerBot:fix/eval-grad-view-subarray

Conversation

@MilesCranmerBot
Copy link
Copy Markdown
Contributor

@MilesCranmerBot MilesCranmerBot commented Feb 14, 2026

Fixes a crash when calling eval_grad_tree_array with X::SubArray (e.g. view(dataset.X, :, idx) used by batching/custom losses).

Empirical reproducer (on master):

  • eval_grad_tree_array(tree, view(X, :, idx), options; variable=true) throws a TypeError because we typeassert the gradient to typeof(X).
  • DynamicExpressions returns an Adjoint{T, Matrix{T}} gradient for SubArray inputs.

Change:

  • Relax the expected gradient type in InterfaceDynamicExpressions.eval_grad_tree_array to AbstractMatrix{eltype(X)}.
  • Add a regression test that exercises eval_grad_tree_array on view(X, :, idx) and compares against Zygote.

Related: #362.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 14, 2026

Benchmark Results (Julia v1)

Time benchmarks
master 1b3a01c... master / 1b3a01c...
search/multithreading 14.7 ± 0.16 s 15 ± 0.26 s 0.985 ± 0.02
search/serial 32 ± 0.049 s 31.8 ± 0.18 s 1.01 ± 0.006
utils/best_of_sample 1.74 ± 0.42 μs 1.6 ± 0.4 μs 1.09 ± 0.38
utils/check_constraints_x10 17 ± 4.4 μs 16.9 ± 4.3 μs 1 ± 0.36
utils/compute_complexity_x10/Float64 2.18 ± 0.093 μs 2.15 ± 0.082 μs 1.01 ± 0.058
utils/compute_complexity_x10/Int64 2.08 ± 0.09 μs 2.05 ± 0.09 μs 1.02 ± 0.062
utils/compute_complexity_x10/nothing 1.52 ± 0.08 μs 1.52 ± 0.08 μs 1 ± 0.074
utils/insert_random_op_x10 5.39 ± 1.7 μs 5.4 ± 1.7 μs 0.998 ± 0.45
utils/next_generation_x100 0.435 ± 0.025 ms 0.447 ± 0.025 ms 0.973 ± 0.077
utils/optimize_constants_x10 0.034 ± 0.0075 s 0.0336 ± 0.0073 s 1.01 ± 0.31
utils/randomly_rotate_tree_x10 8.33 ± 0.96 μs 8.42 ± 0.96 μs 0.989 ± 0.16
time_to_load 2.59 ± 0.0098 s 2.63 ± 0.039 s 0.982 ± 0.015
Memory benchmarks
master 1b3a01c... master / 1b3a01c...
search/multithreading 0.204 G allocs: 52.5 GB 0.205 G allocs: 53.4 GB 0.983
search/serial 0.207 G allocs: 53.8 GB 0.207 G allocs: 53.8 GB 1
utils/best_of_sample 0.038 k allocs: 3.25 kB 0.038 k allocs: 3.25 kB 1
utils/check_constraints_x10 0.034 k allocs: 0.875 kB 0.034 k allocs: 0.875 kB 1
utils/compute_complexity_x10/Float64 0 allocs: 0 B 0 allocs: 0 B
utils/compute_complexity_x10/Int64 0 allocs: 0 B 0 allocs: 0 B
utils/compute_complexity_x10/nothing 0 allocs: 0 B 0 allocs: 0 B
utils/insert_random_op_x10 0.046 k allocs: 1.94 kB 0.045 k allocs: 1.88 kB 1.03
utils/next_generation_x100 4.63 k allocs: 0.276 MB 4.63 k allocs: 0.276 MB 1
utils/optimize_constants_x10 24.5 k allocs: 25.4 MB 21.5 k allocs: 21.3 MB 1.19
utils/randomly_rotate_tree_x10 0.042 k allocs: 1.34 kB 0.042 k allocs: 1.34 kB 1
time_to_load 0.145 k allocs: 11 kB 0.145 k allocs: 11 kB 1

@MilesCranmerBot MilesCranmerBot force-pushed the fix/eval-grad-view-subarray branch from f38398c to 180a4b3 Compare March 11, 2026 00:19
@MilesCranmerBot
Copy link
Copy Markdown
Contributor Author

Resumed this branch and revalidated it against current master.

What changed:

  • keep eval_grad_tree_array return type concrete by normalizing the gradient to Matrix{eltype(X)}
  • preserve the existing regression test for SubArray batched inputs

Local verification:

  • minimal batched-view reproducer now returns Matrix{Float32} gradient and succeeds
  • TEST_GROUP=integration/ad/zygote Pkg.test(coverage=true) passes
  • full local Pkg.test() passes (1464 tests)

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@MilesCranmerBot MilesCranmerBot force-pushed the fix/eval-grad-view-subarray branch from 180a4b3 to 1b3a01c Compare April 11, 2026 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant