Reopen: Tomographic pseudo-cl

[sachaguer]

Reopened tomographic pseudo-cl PR #221 after merging by mistake. (related to issue #214

The updated pseudo-cl script are running end-to-end without crashing on the GLASS mock.
The code still needs to be validated and previously requested unit tests to be implemented before merging.

[sachaguer]

I think this is ready for a final (?) review @cailmdaley @martinkilbinger. I implemented unit tests. I ran the code on the GLASS mock and checked that the measurement matches the input Cls computed with CAMB. See the plot below, it seems to do the job. Note that this is also a nice validation for the GLASS mock branch!

The remaining test is whether the iNKA covariance implementation is correct. This could be long maybe we can merge this PR and I will open an issue if I spot bugs while doing the validation.

[cailmdaley]

Nice job Sacha! Claude review incoming. also note the conflicts

[cailmdaley]

Really nice arc from #221 to here, Sacha — all four bugs from the last review are fixed (pol_factor with its assert, the tomo/"all" branch logic now cleanly factored into _get_tomographic_bin, np.integer, tomo_bin_col threaded through get_params_rho_tau), the new tests genuinely exercise the cross-bin seam, and the GLASS-vs-CAMB agreement is the validation that matters most. The request-changes below is dominated by one git accident and one crash, both quick to fix.

1. The merge trap (blocker — nothing else matters until this is fixed)

Because #221's merge was reverted on the base branch (#239 / f499ac8) after this branch diverged, git considers the implementation "already merged" — so merging this PR as-is would silently keep the reverted pseudo_cl.py, landing your tests without the functions they test. This isn't hypothetical; I simulated the merge:

git merge-tree --write-tree feature/sp_validation-extend-to-tomography feature/tomo-pseudo-cl
# → cosmo_val/pseudo_cl.py in the result: 877 lines (the reverted version)
#   on this branch:                        1433 lines (the implementation)

It's the classic "undoing a faulty merge" scenario, and the fix is mechanical, on feature/tomo-pseudo-cl:

git merge origin/feature/sp_validation-extend-to-tomography   # resolve the 2 small conflicts toward your side
git revert f499ac8d   # revert the revert — restores the implementation as new commits

Nice side effect: GitHub will then show the true ~2200-line diff instead of the current 3-file mirage, so the implementation is actually reviewable in place.

2. Crash in the non-tomographic covariance merge (blocker)

cosmo_val/pseudo_cl.py, _merge_iNKA_covariance, else branch (~line 1081): bin_key_a1/a2/b1/b2 only exist inside the tomography branch, so any compute_tomography=False run ends in NameError. Should be tomo_bin_quad=("all", "all", "all", "all"). (Since these lines aren't in the visible diff yet, file:line pointers here instead of inline — one more argument for fixing item 1 first.)

3. Small things in the covariance orchestrator (should-fix, same file)

• ~line 1027: leftover print(self._pseudo_cls[ver]["tomo_bin_all_tomo_bin_all"]) — and note the tomo merge path also reads that key for n_ell, so a tomography-only run hard-depends on the non-tomo spectrum having been computed. Worth a comment if that coupling is intended.
• ~line 226: if not f"W{bin_key1}" not in n_gal_map_dict: — double negation, so this store only fires when the key already exists. Currently rescued by the auto pairs populating every bin via the bin_key2 line below it, but it's a landmine.
• ~line 314: input_cl_a1_b2 guards its lookup with bin_key_a1 <= bin_key_a2 where the key being built is W{a1}xW{b2} — should be <= bin_key_b2. Unreachable today (ordered pair enumeration guarantees a1 ≤ b1 ≤ b2) but wrong as written.
• Question, not a demand: the data vector debiases with get_noise_bias_from_gaussian_real (hardcoded), while the covariance fiducial goes through get_noise_bias honoring noise_bias_method (default analytic) — so DV and covariance use different noise models by default. Intentional?

What I checked and found right

Since the iNKA machinery is the part you flagged as still-to-validate: I verified the NmtCovarianceWorkspace.from_fields(a1, a2, b1, b2) field ordering, the (cl_a1b1, cl_a1b2, cl_a2b1, cl_a2b2) ordering, and the wsp_a/wsp_b pairing against the installed pymaster docstring — all exactly NaMaster's documented convention. The coupled-Cl ÷ ⟨mask_a·mask_b⟩ normalization matches the iNKA prescription (Nicola et al. 2020, Eq. 2.36) too. The weighted-mean shear map (∑we/∑w) and the auto-only noise subtraction are also correct. So the follow-up validation issue you proposed is about confirming numbers, not hunting for structure — happy with merging on that plan once the blockers land.

Three test-side suggestions inline. Thanks for the careful reopen instead of force-pushing over the revert — that's what made the trap visible at all.

— Claude (Fable), on behalf of Cail

[cailmdaley]

This docstring is now false — the helper no longer replicates the map construction, it delegates to cv.get_shear_map, so the map math has lost its one independent oracle (the old bincount/divide-by-weight-sum replica). The downstream numeric pins still backstop it indirectly, so I won't insist — but I'd mildly prefer restoring the independent replica; failing that, at least fix the docstring so nobody mistakes this for a cross-check.

[sachaguer]

I rather added a test of the functions building the shear maps.

[sachaguer]

I should have accounted for all comments correctly. The merge part was tricky I hope I did not mess anything but the unit tests are passing so it must be correct.

[cailmdaley]

Thanks Sacha LGTM!