Add tomography to the theory prediction.#79
Conversation
…ests for the tomographic case.
cailmdaley
left a comment
There was a problem hiding this comment.
Thanks Sacha — this is the right shape: per-bin tracers, W{i}xW{j} keys matching CAMB's own convention, combinations_with_replacement for the auto+cross pairs, and genuinely good test coverage including the CCL↔CAMB tomo consistency check. A few comments below, one design question, and one physics question about a test tolerance.
The design question: return type. Both functions now return a dict always, even for single-bin input — every existing caller breaks (the test named test_backwards_compatibility had to be edited to append ["W1xW1"], which is a nice tell that it no longer is 🙂). Two options:
- dict-always (as written): uniform API, no polymorphic return.
- array for 1-D
nz, dict for 2-D: zero downstream churn, but a polymorphic return is a long-term tax.
Our vote is (1) — we're happy to break the old signature as long as the change is propagated so all consumers pass. Concretely that means updating the sp_validation call sites in the same window (heads-up: b_modes.py does np.concatenate(get_theo_xi(...)), which needs np.concatenate(get_theo_xi(...)["W1xW1"]) or similar). If we go that way, I'd also rename/drop the test_backwards_compatibility* tests — there's no backwards compatibility left to test, and pretending otherwise is more confusing than the clean break. Your call though!
Everything else is line comments — nothing blocking once the return-type question is settled.
— Fable, on behalf of Cail
| Redshifts for n(z) distribution | ||
| nz : array | ||
| n(z) redshift distribution | ||
| n(z) redshift distribution. If nz.shape[1] > 1, assumes multiple tomographic bins. |
There was a problem hiding this comment.
Worth documenting the accepted shapes explicitly here: (n_z,) or (n_z, n_tomo_bins) — and, importantly, that the return is always a dict keyed "W{i}xW{j}" (i ≤ j, 1-indexed), even for a single bin. As written ("If nz.shape[1] > 1…") a reader could think 1-D input still returns an array. Same applies to the get_theo_xi docstring below (its Returns section should also say the values are (xi_p, xi_m) tuples).
| cl : array | ||
| Angular power spectrum | ||
| cl : dict | ||
| Angular power spectrum in dictionnary indexed by the tomographic bin keys. |
There was a problem hiding this comment.
typo: dictionnary → dictionary
| wa=wa, | ||
| ) | ||
|
|
||
| n_tomo_bins = nz.shape[1] if len(nz.shape) > 1 else 1 |
There was a problem hiding this comment.
Cheap fail-fast guard worth adding here: a transposed nz — (n_bins, n_z) instead of (n_z, n_bins) — would run fine and give silently wrong physics. One line catches it:
assert nz.shape[0] == len(z), f"nz first axis ({nz.shape[0]}) must match z ({len(z)})"| # Create lensing tracer | ||
| lens = ccl.WeakLensingTracer(cosmo, dndz=(z, nz)) | ||
| for bin_key in range(1, n_tomo_bins + 1): | ||
| tracers[f"W{bin_key}"]= ccl.WeakLensingTracer(cosmo, dndz=(z, nz[:, bin_key - 1])) |
There was a problem hiding this comment.
missing space before = — ruff would fix this 🙂 (the loop could also be a dict comprehension, but take or leave)
| pars.SourceWindows = [ | ||
| camb.sources.SplinedSourceWindow(z=z, W=nz, source_type="lensing") | ||
| ] | ||
| if len(nz.shape) == 1: |
There was a problem hiding this comment.
This branch is unreachable: nz was already promoted to 2-D at the top of the function (nz[:, np.newaxis]), so len(nz.shape) == 1 is never true here. The list-comprehension branch below handles the single-bin case fine — this if/else can collapse to just the comprehension.
| npt.assert_allclose(xim1[small_scale_mask], xim2[small_scale_mask], rtol=0.18) | ||
|
|
||
| if np.any(large_scale_mask): | ||
| npt.assert_allclose(xim1[large_scale_mask], xim2[large_scale_mask], rtol=0.18) |
There was a problem hiding this comment.
Question about the 0.18 before it lands: the non-tomo version of this test uses scale-dependent tolerances (0.10 small-θ, 0.05 large-θ), and there's a real physical reason ξ⁻ is the fragile one — it's a J₄ transform, so its support sits at systematically higher ℓ than ξ⁺, and truncating at the coarse ℓ_max=1000 chops real signal. Tomography plausibly makes that worse for W2xW2 specifically (higher-z sources → C_ℓ support at higher ℓ → more power past the cutoff).
So: is the 0.18 driven by W2xW2 at small θ? If yes it's genuine truncation error and fair — but then the flat 0.18 at large θ (where the non-tomo test holds 0.05) is looser than it needs to be, and per-key or per-scale tolerances (or a higher coarse ℓ_max) would keep the test actually constraining. If W1xW1 also needs 0.18, something else is going on and worth a look.
There was a problem hiding this comment.
None of the above. I did not see why the tolerance should change with tomography albeit the deeper redshift distribution. It was a typo from my side. With the same tolerance than the non-tomo case the CI pass.
Summary
The existing script to compute theory predictions have been updated to take as input multiple n(z), allowing to efficiently compute tomographic angular cell and xi.
The typing of the functions have changed and this might need to be propagated before merging.
See issue #231 in
sp_validationReviewer Checklist