Add SchechterMag distribution for astrophysical luminosity functions#2213
Add SchechterMag distribution for astrophysical luminosity functions#2213arham766 wants to merge 4 commits into
Conversation
|
@alserene, would you mind helping us review this one? Especially in math, because I do not know much about astrophysics XD |
|
These are some comments from a Claude-assisted review 1. Gradients are silently wrong at exactly Suggested fixes, in order of preference:
2. One-line fix: move 3. Sampling is ~60x slower than it needs to be (medium). 4. float32 failure mode returns 5. Minor items.
|
|
Thanks @juanitorduz — I validated each point locally before changing anything. Results: 1. Pole gradients — confirmed, fixed. Reproduced (worse than your numbers with a batch of values: autodiff gave the same 2. 3. Sampling speed — confirmed (67x vs 4. float32 underflow — confirmed ( 5. Full battery after the changes: 117 passed / 50 skipped; lint/format clean. |
|
Thanks @arham766 ! Here is another review round (pair review with Claude)
|
|
Thanks — validated and fixed. Confirmed both behaviors first:
Full |
|
Thanks for your work on this @arham766, it's great to see this happen! I've compared it against the standalone implementation I originally wrote, and the mathematics looks sound 👍 One suggestion from an astronomy perspective: would it be possible to expose the normalisation integral (or an equivalent helper) through the public API? I agree that the Poisson count term belongs in the user's model rather than in Also, my original proposal included both single- and double-Schechter distributions. I'd certainly welcome a follow-up PR adding a |
|
Thanks so much for checking the math against your implementation, @alserene — that's exactly the review this needed! Both suggestions taken:
|
Adds a
SchechterMagdistribution — the single Schechter luminosity function (Schechter 1976) in absolute-magnitude space, truncated to[low, high]— as discussed in #2065.Design
f(m | alpha, m_star) ∝ x^(alpha+1) exp(-x)withx = 10^(-0.4 (m - m_star)); equivalently a doubly-truncated Gamma under the change of variables, which is what makes the density normalizable for faint-end slopesalpha <= -1(the empirically common regime).jax.scipy.special.gammainccdoesn't support — implemented via the standard downward recurrence fromGamma(a+3, x), with the removable singularities ata ∈ {0, -1, -2}patched through the exponential integral. Valid foralpha > -4, which covers any physically sensible slope.samplevia inverse-CDF (64-iteration bisection,custom_jvpwith implicit differentiation ofcdf(icdf(q)) = q), so samples are reparametrized and usable in gradient-based inference.phi_starintentionally omitted: the amplitude doesn't affect the normalized density (docstring points users to a Poisson term on counts for number-density inference).Tests (in the generic harness + dedicated):
CONTINUOUS→ full generic battery: 92 passed, 40 skipped (skips are the standard pytree/expand exclusions applied to all distributions).log_probandcdfvalidated againstscipy.integrate.quadreferences acrossalpha ∈ {-2.5, -2.0, -1.25, -1.0, -0.5, 0.7}.alpha ∈ {-1, -2, -3}).make lintandmake format --checkare clean.Happy to follow up with the linear-luminosity-space counterpart and/or a double-Schechter if there's interest — kept this PR to the magnitude form since that's what survey likelihoods typically parametrize.
Closes #2065