Skip to content

Unify the gp2Scale covariance computation behind one primitive - #35

Merged
MarcusMNoack merged 1 commit into
masterfrom
gp2scale-covariance-unification
Aug 7, 2026
Merged

Unify the gp2Scale covariance computation behind one primitive#35
MarcusMNoack merged 1 commit into
masterfrom
gp2scale-covariance-unification

Conversation

@MarcusMNoack

Copy link
Copy Markdown
Collaborator

The prior covariance, the blocks added by an append, and the posterior's cross-covariance were three different code paths at three levels of support: two near-duplicate distributed functions in GPprior, and nothing at all for the posterior, which called the kernel directly and so built a dense (N x n_pred) array on the client.

They are all the same operation over a block grid, differing only in whether the two point sets are the same. distributed_covariance in the new gp2Scale_covariance module is that operation; GPprior._gp2Scale_covariance is its only caller and owns scatter lifetime and nothing else.

Falling out of the unification:

  • A four-argument, args-taking kernel now works under gp2Scale. The old workers called kernel(x1, x2, hps) unconditionally, so a signature supported everywhere else in fvGP raised TypeError on the worker.
  • The posterior cross-covariance is distributed and stays sparse, so posterior_mean never materializes (N x n_pred). posterior_covariance cannot avoid a dense solve, KV^-1 being dense whatever KV is, so it chunks over prediction points to cap the intermediate at (N x chunk).
  • The joint-covariance methods (joint_gp_prior, gp_mutual_information, gp_total_correlation) are dense in N by construction; they now densify K explicitly and warn, rather than failing somewhere inside numpy.

Add a row-wise distribution alongside the existing block-wise one, selected with gp2Scale_distribution. Row-wise has each worker return a finished CSR row strip, so the COO-to-CSR sort happens in parallel and host assembly is a concatenation with no global COO and no mirroring; it cannot exploit symmetry and so doubles the kernel evaluations. Block-wise remains the default.

Assembly speedups: the per-block diagonal mask was a Python list comprehension over every nonzero, now vectorized (97.6 ms -> 0.34 ms on 1.1M nonzeros, and it ran once per diagonal block per likelihood evaluation); empty blocks are skipped rather than shipped; indices are int32 where the matrix allows; the mirrored triplets are written into a single preallocation instead of two np.hstack passes, freeing each gathered block as it is copied; and the augmented matrix is assembled from all-CSR blocks so scipy takes its fast path instead of converting the lot to COO.

Fix the scatter race documented as unavoidable. Dask keys scattered data by a hash of its content, so a second GP on the same data, or a second prediction at the same points, lands on one key and the first copy's release races the second scatter inside the scheduler; the tasks then return CancelledError or KeyError. Every scatter now uses hash=False and so gets its own key. _harvest also raises on an exception result rather than letting it reach the assembly and fail there as an unrelated type error.

The prior covariance, the blocks added by an append, and the posterior's
cross-covariance were three different code paths at three levels of support:
two near-duplicate distributed functions in GPprior, and nothing at all for the
posterior, which called the kernel directly and so built a dense (N x n_pred)
array on the client.

They are all the same operation over a block grid, differing only in whether
the two point sets are the same. distributed_covariance in the new
gp2Scale_covariance module is that operation; GPprior._gp2Scale_covariance is
its only caller and owns scatter lifetime and nothing else.

Falling out of the unification:

* A four-argument, args-taking kernel now works under gp2Scale. The old workers
  called kernel(x1, x2, hps) unconditionally, so a signature supported
  everywhere else in fvGP raised TypeError on the worker.
* The posterior cross-covariance is distributed and stays sparse, so
  posterior_mean never materializes (N x n_pred). posterior_covariance cannot
  avoid a dense solve, KV^-1 being dense whatever KV is, so it chunks over
  prediction points to cap the intermediate at (N x chunk).
* The joint-covariance methods (joint_gp_prior, gp_mutual_information,
  gp_total_correlation) are dense in N by construction; they now densify K
  explicitly and warn, rather than failing somewhere inside numpy.

Add a row-wise distribution alongside the existing block-wise one, selected
with gp2Scale_distribution. Row-wise has each worker return a finished CSR row
strip, so the COO-to-CSR sort happens in parallel and host assembly is a
concatenation with no global COO and no mirroring; it cannot exploit symmetry
and so doubles the kernel evaluations. Block-wise remains the default.

Assembly speedups: the per-block diagonal mask was a Python list comprehension
over every nonzero, now vectorized (97.6 ms -> 0.34 ms on 1.1M nonzeros, and it
ran once per diagonal block per likelihood evaluation); empty blocks are
skipped rather than shipped; indices are int32 where the matrix allows; the
mirrored triplets are written into a single preallocation instead of two
np.hstack passes, freeing each gathered block as it is copied; and the
augmented matrix is assembled from all-CSR blocks so scipy takes its fast path
instead of converting the lot to COO.

Fix the scatter race documented as unavoidable. Dask keys scattered data by a
hash of its content, so a second GP on the same data, or a second prediction at
the same points, lands on one key and the first copy's release races the second
scatter inside the scheduler; the tasks then return CancelledError or KeyError.
Every scatter now uses hash=False and so gets its own key. _harvest also raises
on an exception result rather than letting it reach the assembly and fail there
as an unrelated type error.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.44068% with 40 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.31%. Comparing base (51d1676) to head (c781dd8).

Files with missing lines Patch % Lines
fvgp/gp2Scale_covariance.py 72.65% 35 Missing ⚠️
fvgp/gp_posterior.py 93.93% 2 Missing ⚠️
tests/test_fvgp.py 97.93% 2 Missing ⚠️
fvgp/gp_prior.py 97.29% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #35      +/-   ##
==========================================
+ Coverage   83.14%   83.31%   +0.17%     
==========================================
  Files          19       20       +1     
  Lines        6827     7017     +190     
==========================================
+ Hits         5676     5846     +170     
- Misses       1151     1171      +20     

☔ 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.

@MarcusMNoack
MarcusMNoack merged commit 0cf41e3 into master Aug 7, 2026
10 of 17 checks passed
@MarcusMNoack
MarcusMNoack deleted the gp2scale-covariance-unification branch August 7, 2026 01:05
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