Skip to content

Add tomographic and 3x2pt covariance realspace measurement - #256

Open
LisaGoh wants to merge 11 commits into
developfrom
feature/tomo-real-space
Open

LisaGoh wants to merge 11 commits into
developfrom
feature/tomo-real-space

Conversation

@LisaGoh

@LisaGoh LisaGoh commented Jul 10, 2026 •

Copy link
Copy Markdown
Member

Replaces CosmoCov with OneCovariance for the real-space cosmic-shear ξ± covariance, and adds a tomographic variant. This PR only touches the covariance. The tomographic data vector, and putting this covariance into SACC, are in #306.

Brought up to date with develop (merged, not rebased; T is retired since #402).

What changes

  • workflow/rules/covariance.smk
    • covariance_ini writes the OneCovariance .ini; covariance_run runs it and writes a dense ξ± matrix (.dat, np.savetxt): G+NG for ng, G only for g.
    • Survey parameters come from the catalogue entry (cov_th: sigma_e, n_e, A). The n(z) comes from shear.redshift_path, and the mask Cℓ from MASK_CLS_FILES; both are split into directory and file, as OneCovariance expects.
    • Targets: covariance, covariance_masked, covariance_unmasked (NG on the reporting grid, G on the integration grid), and the new covariance_tomo.
    • Removed: the CosmoCov rules (covariance_ini/covariance_cosmocov/covariance_cat/covariance_process) and workflow/scripts/cosmocov_process.py.
  • workflow/common.py
    • One naming scheme, COV_BASE_TEMPLATE, used for rule outputs through cov_output() and filled in by covariance_base():
      covariance_{version}_{gaussian}_minsep=…_maxsep=…_nbins=…{mask_suffix}{tomo_suffix}
    • covariance_base / covariance_dir / covariance_path take tomo_suffix. covariance_path() now returns the .dat by default, so inference, the glass mocks, cosmo_val (pure-E/B, COSEBIs, assemble_sacc) and the bmodes figures read the OneCovariance matrix without changes of their own.
    • Wildcard constraints for tomo_suffix ((_tomo)?) and npatch.
  • workflow/rules/masks.smk: removed prepare_mask_for_cosmocov.
  • Paper configs: tools.cosmocov_executable is replaced by tools.onecov_executable.
  • Tests: removed the CosmoCov post-processing regression test; the tau-sampling and glass-covariance checks stay.

Tested

  • Dry runs resolve for covariance, covariance_tomo and the whole cosmo_val paper DAG. For the fiducial catalogue, the rendered .ini matches its cov_th and n(z).
  • pytest: covariance-rule regression, assemble_sacc, xi-grid and cv-params tests pass.

Heads-up for reviewers

  • OneCovariance products use the same directory and file names CosmoCov used. Launch with your own COSMO_INFERENCE; on the shared tree the new .ini would overwrite CosmoCov's.
  • No .dat exists yet, so anything that needs a covariance triggers OneCovariance runs. The non-Gaussian ones are slow.
  • The bmodes paper scripts that hard-code _processed.txt still read the published CosmoCov files, so the paper's numbers are unchanged.
  • To check before merging: the element order of OneCovariance's matrix output (expected: ξ+ then ξ−, by θ; for tomography, bin pairs (1,1), (1,2), …, (n,n)) against an existing _processed.txt.
  • Tomographic inputs: an n(z) file with one column per bin, and comma-separated per-bin values in cov_th. With today's non-tomographic fiducial catalogue, covariance_tomo only renames the single-bin covariance.

Out of scope

  • ggl / 3x2pt covariances: deferred until there is a lens sample.

🤖 Generated with Claude Code

@LisaGoh LisaGoh linked an issue Jul 10, 2026 that may be closed by this pull request
@LisaGoh LisaGoh self-assigned this Jul 10, 2026
@github-actions

github-actions Bot commented Jul 10, 2026 •

Copy link
Copy Markdown
Contributor

🤖 autofix pushed 74caf77, ruff is clean — formatting and safe lint fixes were applied for you; nothing else to do.

@LisaGoh
LisaGoh force-pushed the feature/tomo-real-space branch from d235788 to 00a0f6b Compare July 10, 2026 16:17
@cailmdaley

Copy link
Copy Markdown
Collaborator

thank you Lisa this looks great, please continue to take liberties! review incoming.

for the n(z) format, i'm proposing we migrate to SACC (see #241 for motivation, #246 for what this would mean for the OneCovariance interface). but i think it's fine to go ahead with what you've assumed for now so this PR can be merged quickly, and we can do the switch to SACC later.

@cailmdaley cailmdaley left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Lisa — this is a really nice move. The CosmoCov→OneCovariance swap is much cleaner than the old block/cat/process chain, and the COV_BASE_TEMPLATE consolidation plus workflow.source_path fix are genuine improvements to common.py, liberties happily taken. A few things to sort before merge, one of which I think breaks the workflow as-is:

Blocker: build_redshift_path is deleted but still called. workflow/rules/inference.smk line 66 and (worse) line 138 still call build_redshift_path — line 138 is evaluated at parse time, so any snakemake invocation on this branch should die with a NameError, not just covariance targets. Simplest fix: keep build_redshift_path as it was (it's exactly build_redshift_dir(version) + build_redshift_path_lens(version, blind)) alongside the new helpers, or update inference.smk in this PR. (If the covariance run worked for you, I suspect it was against a locally-edited inference.smk — worth double-checking what state that was in.)

Personal-path defaults — let's fix the pattern, not the username. To be fair, the previous defaults were no more neutral: they were Cail's personal tree. Rather than swapping one username for another, let's use this PR to drop personal defaults entirely — make COSMO_VAL / COSMO_INFERENCE / CAT_CONFIG required (env var or config key, loud error if unset), so each of us sets our own and the repo carries nobody's paths. Happy for that to be a small follow-up if you'd rather keep this PR focused. The one that genuinely needs reverting here is COSMOLOGY_PARAMS — inline comment explains why (it's about the generating rule, not whose disk).

Rest is inline — mostly small. On n(z) formats and area/sigma_e specification: as in my comment above, fine to keep your assumed formats for now and revisit with the SACC migration (#241/#246).

— Fable on behalf of Cail

Comment thread workflow/common.py Outdated
os.environ.get(
"COSMO_VAL", "/n17data/cdaley/unions/code/sp_validation/cosmo_val/output"
"COSMO_VAL",
"/n23data1/n06data/lgoh/scratch/UNIONS/sp_validation/results/cosmo_val",

@cailmdaley cailmdaley Jul 11, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than these defaults carrying anyone's personal tree (the old values were Cail's, so this is a fix-the-pattern note, not a revert request): suggest requiring COSMO_VAL / COSMO_INFERENCE / CAT_CONFIG explicitly — env var or config key, loud error if unset — so each of us sets our own and the repo carries nobody's paths. Fine as a small follow-up if you'd rather keep this PR focused.

Comment thread workflow/common.py Outdated
# Regenerate with: snakemake results/cosmology/planck18.json
# Resolved relative to the run directory at configure() time.
COSMOLOGY_PARAMS = "results/cosmology/planck18.json"
COSMOLOGY_PARAMS = "/n23data1/n06data/lgoh/scratch/UNIONS/sp_validation/results/cosmology/planck18.json"

@cailmdaley cailmdaley Jul 11, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one does need to stay relative (independent of the defaults question above): the cosmology_params rule generates this file at the relative path results/cosmology/planck18.json in the run directory, so pointing the constant at an absolute path decouples the reader from the generator — the rule would write one file and everything else would read another.

Comment thread workflow/common.py Outdated
base_version = re.sub(r"_ecut\d+", "", base_version)
if "v1.4.11" in base_version:
base_version = "SP_v1.4.6"
return f"nz_{base_version}_{blind}_source.txt" # TO DO: check lens and source file format

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three copies of the same version-normalization regex block now — worth one _nz_base_version(version) helper that the three functions share.

Also a naming/semantics check to resolve with the TO-DO: in the ini, zlens_file gets build_redshift_path_lens (the existing source-galaxy n(z)) and zclust_file gets ..._source.txt. OneCov's zlens is the lensing source sample and zclust the clustering lens sample — so a file suffixed _source feeding zclust reads inverted. If the intent is _lens.txt for the clustering sample, let's fix the suffix before anyone generates files against this convention.

Minor: build_redshift_dir returns a trailing-slash dir which the rule then passes through os.path.dirname, and the _lens/_source helpers return bare filenames that go through os.path.basename. Both round-trips can go — return dir and filenames and use them directly.

Comment thread workflow/rules/covariance.smk Outdated
cov_th = config[base_version]["cov_th"]
return cov_th["A"], cov_th["n_e"], cov_th["sigma_e"]

if config["probe_3x2pt"] == "wl":

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branches on the global config["probe_3x2pt"] while everything else in the rule branches on the {probe} wildcard — if they disagree (config says 3x2pt, someone requests a wl target, or vice versa) you get an ini with mismatched survey-spec lines. Suggest get_cat_params(version, probe) taking the wildcard, and dropping the global key entirely: then covariance_3x2pt (line 391) can just say probe="3x2pt", which also fixes the slightly bewildering probe_3x2pt: wl in the test config.

Comment thread workflow/rules/covariance.smk Outdated
small_k_damping_for1h = damped

[misc]
num_cores = 8

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoded 8 here has to match threads: 8 on covariance_onecov by hand (the ini is written by a different rule, so {threads} can't be used directly). Worth a single shared constant (e.g. ONECOV_CORES = 8) used by both, so they can't drift.

Comment thread workflow/rules/covariance.smk Outdated
rules.covariance_ini_onecov.output,
output:
str(COSMO_INFERENCE / "data/covariance/covariance_{version}_{blind}_{gaussian}_minsep={min_sep}_maxsep={max_sep}_nbins={nbins}{mask_suffix}/cov_tmp_ssss_{block_pm}_cov_Ntheta{nbins}_Ntomo1_{block_i}")
# Matches `[output settings] file = cov_tmp_onecov.dat` in the ini.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment — the ini's file = is now the full covariance base name, not cov_tmp_onecov.dat.

Comment thread workflow/rules/covariance.smk Outdated
)
GLASS_MOCK_COV_DIR = config.get("glass_mocks", {}).get(
"covariance_dir",
"/automnt/n17data/cdaley/unions/pure_eb/results/covariance/glass_mock_v1.4.6",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making this config-overridable is a nice touch. The default uses the deprecated pure_eb/ compat symlink though — canonical path is now /automnt/n17data/cdaley/unions/analyses/shear_2d/bmodes_2d/results/covariance/glass_mock_v1.4.6.

Comment thread workflow/rules/covariance.smk Outdated


def fiducial_covariance_outputs(mask_suffix=""):
def fiducial_covariance_outputs(mask_suffix="", probe=DEFAULT_PROBE):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probe=DEFAULT_PROBE binds the default at function-definition time — it happens to work because configure() runs before the includes, but it's fragile ordering. Safer: probe=None and resolve inside (which is also the pattern covariance_base already uses). Relatedly, DEFAULT_PROBE = "wl" hardcoded inside configure() could just be a module-level constant, or read from config if we ever want a non-wl default.

@LisaGoh
LisaGoh changed the base branch from develop to feature/sp_validation-extend-to-tomography July 24, 2026 15:38
@LisaGoh
LisaGoh force-pushed the feature/tomo-real-space branch from 2c3b003 to 54f5783 Compare July 27, 2026 14:58
@LisaGoh
LisaGoh marked this pull request as ready for review October 9, 2026 10:13
@LisaGoh
LisaGoh changed the base branch from feature/sp_validation-extend-to-tomography to develop October 20, 2026 09:57
@LisaGoh

LisaGoh commented Oct 20, 2026 •

Copy link
Copy Markdown
Member Author

@cailmdaley I think this is ready to be merged! I updated the PR description, rebased it to merge into develop and proposed an updated workflow (see the latest comment in #375). Once this is merged, we can merge #306 as well!

This branch has not been deployed

No deployments
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.

Tomographic real space measurement

2 participants