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:
|
|
Late to the party, but ... a couple of user-level comments ... On the domain for the power-law index a — for a < -1 the indefinite integral (i.e. the total population number) diverges; for -1 < a < -2 the number diverges but the integrated light does converge. For this reason, and for mine, i would say that a > -2 is a sensible physical limit for astronomical purposes. That said, the difference between limits of -1, -2, -3, ... is just the number of iterations using the recursion relation. This might be easily accommodated by including a 'num_recursions' keyword argument? Then you can have a limit of, say, -2 by default, but with the facility for users to explore more extreme solutions if they want to (at the cost of extra compute time). |
|
Thanks @entaylor — good points, and worth spelling out the reasoning in the code's terms. On the domain: agreed that for physical luminosity/mass functions alpha > -2 is where sensible fits live (and untruncated, the number integral diverges below -1). The reason the class allows more is that this distribution is defined on a truncated interval, so it's a proper, normalizable distribution for any alpha the incomplete-gamma evaluation can handle — the On |
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