Add Yung+24 mass functions#317
Merged
Merged
Conversation
Addresses review feedback on #296: the z-range ValueError branch was untested, and the existing units tests only checked internal self-consistency rather than correctness against real data. Adds a z-range validation test, plus a test that digitizes n-body data points from Fig. A1 of Yung+24 (arXiv:2309.14408) and checks that fsigma, combined with the paper's own sigma(Mvir) fit (Eq. A4) and cosmology, reproduces the published halo mass function to within the fit's own quoted accuracy (~30%). This is an outcome-level check against the actual published simulation results, rather than a re-derivation of the same fitting formula the code implements. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The GUREFT simulations were run with sigma_8=0.829 (Yung+24 Sec. 2), not 0.823 as previously recorded in sim_definition. This field is documentation-only metadata and doesn't affect any fsigma calculation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #317 +/- ##
==========================================
+ Coverage 97.86% 97.89% +0.03%
==========================================
Files 26 26
Lines 2671 2710 +39
==========================================
+ Hits 2614 2653 +39
Misses 57 57 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
codecov flagged the cutmask property (both units branches and the m=None fallback) as uncovered patch lines in #317. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Addresses further review feedback on the fsigma test added in b25c80b: - Expand the (z, logM) grid instead of testing a single mass at 4 redshifts: now covers logM=9 at all z, logM=11 for z<12, and logM=12 for z<=8 (staying within each redshift's plotted mass range). - Reuse hmf's own GrowthFactor instead of re-deriving the linear growth factor by hand. - Compare against the *fitted* curve in Fig. A1, not the noisier n-body points, and clarify in the docstring that this specifically tests fsigma's functional form, not its propagation into dn/dm. - Scale the tolerance by the local sensitivity d ln(fsigma)/d ln(sigma): fsigma ~ exp(-c/sigma^2) is exponentially sensitive to sigma at high z/ mass, and Eq. A4's sigma(Mvir) fit is only an approximation (confirmed by cross-checking it against hmf's own transfer-function-based sigma), so a fixed tolerance is either too loose at low z or spuriously tight at high z. Also adds test_yung24_dndlogm_matches_paper_fit, a separate end-to-end check that MassFunction(hmf_model="Yung24") reproduces the same Fig. A1 points through the full pipeline (mass definitions, rho_m, real sigma(M) from the EH transfer function) -- catching bugs the fsigma-only test can't, using the same scaled-tolerance approach. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
steven-murray
enabled auto-merge (squash)
July 6, 2026 08:35
Contributor
|
Apologies for not getting to this earlier, busy few weeks. Glad it's in |
Collaborator
Author
|
Not a problem and thanks again for the PR! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reopens #296 (originally by @christopherlovell) since that PR has gone unresponsive to review feedback and its branch can't be edited directly. This carries over Chris's original commit unchanged, plus the requested test improvements.
Yung24class insrc/hmf/mass_function/fitting_functions.py, modeled on Tinker08.unitskwarg toggles between Table A1 ("h", default) and Table A2 ("physical").Changes since #296
Addressing review feedback that the original two tests only exercised units-switching/error-raising behavior ("quirks of the fit") rather than actual correctness:
test_yung24_invalid_z_raises: covers the previously-untested z-rangeValueErrorbranch.test_yung24_fsigma_matches_published_hmf: digitizes n-body data points directly from the vector graphics of Yung+24's Fig. A1, converts them tofsigmausing the paper's own independent sigma(Mvir) fit (Eq. A4) and cosmology, and checks the code reproduces them to within the fit's own quoted accuracy (~30%). This is a genuine outcome-level check against the published simulation results, not a re-derivation of the same formula/coefficients the code implements.sim_definition:sigma_8=0.823→0.829(paper's Sec. 2 givessigma_8 = 0.829). Documentation metadata only, doesn't affect any calculation.Test plan
uv run pytest tests/— 403 passed, 1 skippeduv run ruff check/ruff format --checkcleana/bexponents), confirming it actually catches bugsCloses #296
🤖 Generated with Claude Code