Skip to content

Define partype on types, mirroring eltype#2072

Open
devmotion wants to merge 5 commits into
masterfrom
dmw/partype
Open

Define partype on types, mirroring eltype#2072
devmotion wants to merge 5 commits into
masterfrom
dmw/partype

Conversation

@devmotion

@devmotion devmotion commented Jun 22, 2026

Copy link
Copy Markdown
Member

Motivation

In a downstream package I currently maintain an internal _partype that operates on distribution types, so that I can compute the parameter element type of a container of distributions from its eltype — without having to rely on the container being non-empty (i.e. without needing an instance). That internal version is necessarily incomplete: it reimplements per-distribution knowledge that already lives here and can't realistically cover every distribution.

The reason it's needed at all is that partype is currently only defined on instances: every distribution defines partype(d::Dist{T}) = T, with a Float64 fallback, so there's no way to ask partype(Normal{Float32}) for a type — it falls through to the default and silently returns the wrong answer.

This PR makes partype follow the same design as eltype: define it on the type, with the instance method provided for convenience. That makes the type-level query work directly and lets the downstream _partype go away.

Changes

  • partype is now defined on the type, with partype(d) = partype(typeof(d)) forwarding for convenience, documented with a !!! note callout mirroring Base.eltype's docstring.
  • All ~90 distribution methods migrated from partype(d::Dist{T}) to partype(::Type{<:Dist{T}}). Wrapper distributions (Censored, Truncated, AffineDistribution, ReshapedDistribution, OrderStatistic, JointOrderStatistics, MixtureModel, MvLogitNormal) recover the wrapped distribution from their type parameter, exactly as the matching eltype(::Type{...}) methods already do.
  • The generic default changes from Float64 to Real: distribution parameters are <:Real and eltype(Real) === Real, so Real is the natural fallback (and zero/one work on it, unlike Any).
  • Distributions that previously relied on the fallback get an explicit method. This fixes a latent bug where parametric distributions (Dirac, DiscreteNonParametric, Product, UnivariateGMM) reported Float64 instead of their actual parameter type. Parameterless distributions (Chernoff, Kolmogorov) return Union{} — the empty parameter promotion, which is the identity for promote_type and so composes correctly through wrappers; KSDist/KSOneSided return Int (their n), Soliton returns Float64.
  • -(d::UnivariateDistribution) is changed from -one(partype(d)) * d to -one(promote_type(Int, partype(d))) * d so it stays well-typed when partype(d) === Union{} (the multiplier -1 is an Int, and promote_type(Int, Union{}) === Int).

Behavior changes

  • partype(Dirac{Float32}) etc. now return the real parameter type (Float32) instead of Float64.
  • partype(Kolmogorov()) / partype(Chernoff()) return Union{} (were Float64). One consequence: -Kolmogorov() now produces a LocationScale{Int,...} rather than silently a Float64-typed one.
  • The generic fallback for an unknown distribution type is Real (was Float64).

Tests

  • A general invariant wired into test/testutils.jl's test_params, so every distribution that runs the standard battery asserts:

    partype(d) === partype(typeof(d)) === mapreduce(recursive_partype, promote_type, params(d); init = Union{})
  • partype tests live in each distribution's own test file (instance and type-based calls, instance forwarding, wrappers, the bug-fixes, and the Real/Union{} cases), covering the methods the standard battery does not exercise. To run all of them this:

    • enables univariate/discrete/bernoullilogit in runtests.jl (the file existed but was never included);
    • adds minimal test files for Hypergeometric, KSDist, and KSOneSided, which have no entry in the ref-data battery.
  • The generic-mechanism regression tests live in test/univariate/continuous/normal.jl: the Real fallback for bare parametric UnionAlls (partype(Normal)) and abstract hierarchy nodes (partype(UnivariateDistribution)), plus the motivating partype(eltype(container)) for both concrete and abstract element types. A bare UnionAll's type parameters are only upper bounds, so the specific Type{<:Dist{T}} methods don't match and dispatch falls through to the Real default — exactly as eltype(Vector) === Any in Base. These lock that behavior in so a future subtyping change can't silently regress it.

  • Every partype test is wrapped in @inferred, asserting the type-based design stays constant-foldable.

The full test suite passes (4,941,004 passing, 0 failures).

`partype` could previously only be called on instances: every distribution
defined `partype(d::Dist{T}) = T` with a `Float64` fallback, so `partype(SomeDist{T})`
on the type fell through to the default and returned the wrong answer.

Mirror the `eltype` design: define `partype` on the type and provide an instance
method `partype(d) = partype(typeof(d))` for convenience, documented with a
`!!! note` callout. The generic default becomes `Real` (parameters are `<:Real`
and `eltype(Real) === Real`, so `zero`/`one` work on it).

- Migrate all distribution methods from `partype(d::Dist{T})` to
  `partype(::Type{<:Dist{T}})`; wrappers recover the wrapped distribution from
  their type parameter, as the matching `eltype` methods already do.
- Give distributions that relied on the fallback an explicit method, fixing a
  latent bug where parametric distributions (`Dirac`, `DiscreteNonParametric`,
  `Product`, `UnivariateGMM`) reported `Float64`. Parameterless distributions
  (`Chernoff`, `Kolmogorov`) return `Union{}` (the empty parameter promotion);
  `KSDist`/`KSOneSided` return `Int`, `Soliton` returns `Float64`.
- Fix `-(d::UnivariateDistribution)` to `-one(promote_type(Int, partype(d))) * d`
  so it stays well-typed when `partype(d) === Union{}`.
- Add tests, including a general invariant in `testutils`:
  `partype(d) === partype(typeof(d)) === mapreduce(recursive_partype, promote_type, params(d); init=Union{})`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.40%. Comparing base (6b5aa0d) to head (c52648f).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2072      +/-   ##
==========================================
+ Coverage   86.74%   87.40%   +0.65%     
==========================================
  Files         149      149              
  Lines        8882     8891       +9     
==========================================
+ Hits         7705     7771      +66     
+ Misses       1177     1120      -57     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

`partype` defined on types mirrors `eltype`: a specific method like
`partype(::Type{<:Normal{T}}) where {T<:Real}` only matches a concrete
subtype, so a bare UnionAll (`Normal`, whose parameter is only an upper
bound) does not match and dispatch falls through to the generic
`partype(::Type{<:Distribution}) = Real` default, just like `eltype(Vector)`
returns `Any`.

This was correct but untested. Add assertions covering bare parametric
UnionAlls, abstract nodes in the type hierarchy, and the motivating
`partype(eltype(container))` use for both concrete and abstract element
types, so a future subtyping change can't silently regress the fallback.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@devmotion devmotion marked this pull request as ready for review June 23, 2026 08:23
devmotion and others added 3 commits June 23, 2026 19:55
Move the `partype` tests out of `test/utils.jl` and into each
distribution's own test file, covering the methods that the standard
`test_params` battery does not exercise. This fixes the patch-coverage
gap for the newly type-based `partype` methods on `BernoulliLogit`,
`Semicircle`, `PoissonBinomial`, `Soliton`, the deprecated `Product`,
and the unary negation `-(d::UnivariateDistribution)`.

- Enable `bernoullilogit`, `hypergeometric`, `ksdist`, and `ksonesided`
  in `runtests.jl`; the first was never run, the latter three had no
  test file (they are not part of the ref-data battery), so add minimal
  files for them.
- Keep the generic-mechanism tests (the `Real` fallback for abstract
  types and `partype(eltype(container))`) in `normal.jl`.
- Wrap every `partype` test in `@inferred` so the type-based design is
  asserted to stay constant-foldable.

Co-Authored-By: Claude Opus 4.8 (1M context) <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.

1 participant