Skip to content

Merge Sacha's fork with fiducial sp_validation#192

Merged
sachaguer merged 26 commits into
CosmoStat:stub/foundation-mergefrom
sachaguer:develop
Jun 5, 2026
Merged

Merge Sacha's fork with fiducial sp_validation#192
sachaguer merged 26 commits into
CosmoStat:stub/foundation-mergefrom
sachaguer:develop

Conversation

@sachaguer

Copy link
Copy Markdown
Contributor
  • Added the paper plot scripts
  • Added the likelihood config files for the harmonic space analysis
  • Updated the scripts and notebooks I modified

@sachaguer sachaguer requested a review from cailmdaley June 3, 2026 08:14
@sachaguer sachaguer self-assigned this Jun 3, 2026
@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@cailmdaley

Copy link
Copy Markdown
Collaborator

@sachaguer want me to work on the conflicts? or you got it? they seem largely mechanical

@sachaguer

Copy link
Copy Markdown
Contributor Author

I got it but will request your review to make sure I did not break something that was updated since I last pulled the CosmoStat repo

@sachaguer

Copy link
Copy Markdown
Contributor Author

Ready for review @cailmdaley

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

Looks good overall — the bulk is additive (paper plots, harmonic-space likelihood configs, updated notebooks), and the only library-code changes (the camb→ccl backend in cosmology.py, the nside_mask 2¹²→2¹³ bump) read as intended.

CI caught one thing, though. The PR's own checks don't fire yet — its base (stub/foundation-merge) predates the new push-triggered test CI on develop — so I ran the suite by merging current develop into the branch. It fails 23 tests with KeyError: 'mnu' at cosmology.py:283:

mnu = ccl_params.get("mnu", mnu or planck_defaults["mnu"])

planck_defaults (built just above) has no mnu entry, so the fallback raises whenever mnu isn't passed explicitly. Easy fix — the value's already at module level as PLANCK18["m_nu"] = 0.06:

planck_defaults = {
    ...,
    "mnu": PLANCK18["m_nu"],   # PLANCK18 uses m_nu; this lookup uses mnu
}

Good to merge once that's in. Bringing the branch up to develop will also light up CI on the PR itself. Thanks @sachaguer!

— Claude on behalf of Cail

@sachaguer

Copy link
Copy Markdown
Contributor Author

Done

@sachaguer sachaguer merged commit 7da2435 into CosmoStat:stub/foundation-merge Jun 5, 2026
LisaGoh pushed a commit to LisaGoh/sp_validation that referenced this pull request Jul 3, 2026
Fold Sacha's pending foundation (PR CosmoStat#192 head, sachaguer:develop @ c22f075)
onto current develop so the restructuring builds on his foundation without
racing his merge gesture (Cail's direction, 2026-06-05).

.gitignore conflict resolved in favour of develop: kept the .felt tracking
block, rejected sacha's broad cluster bans (*.png *.sh *.fits *.out *.err) —
those get narrowed during the restructuring gitignore pass, not adopted
wholesale. cosmo_val.py / cat_config.yaml auto-merged cleanly (origin's
docstring-RST polish + sacha's functional changes did not collide).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

3 participants