Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC/WIP: coercion "cascades" for MvNormal #1823

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

timholy
Copy link
Contributor

@timholy timholy commented Jan 13, 2024

This redesigns the constructors of MvNormal to:

  • check argument consistency in the inner constructor
  • support "coercion," for example MvNormal{Float32}(μ, Σ)
    will enforce that the result has eltype Float32
  • cascade coercion among the multiple type parameters

The benefits of this design are subtle but significant, as it
allows the user to specify intent, allows the constructors to
accept input arguments that are not (yet) consistent with
the desired output types (e.g., MvNormal{Float64}(Any[1,2], I)),
and makes it easy for developers to "propagate" consistent
types across the code base (e.g., fixes #1544).

Among things this fixes, it's currently possible to construct an MvNormal with inconsistent dimensions:

julia> dist = DiagNormal(zeros(3), PDiagMat(zeros(2)));

julia> rand(dist, 5)
ERROR: DimensionMismatch: Inconsistent argument dimensions.
Stacktrace:

This is possible because the consistency check is not in the inner constructor.

This redesigns the constructors of `MvNormal` to:
- check argument consistency in the inner constructor
- support "coercion," for example `MvNormal{Float32}(μ, Σ)`
  will enforce that the result has eltype `Float32`
- cascade coercion among the multiple type parameters

The benefits of this design are subtle but significant, as it
allows the user to specify intent, allows the constructors to
accept input arguments that are not (yet) consistent with
the desired output types (e.g., `MvNormal{Float64}(Any[1,2], I)`),
and makes it easy for developers to "propagate" consistent
types across the code base (e.g., fixes JuliaStats#1544).
@timholy
Copy link
Contributor Author

timholy commented Jan 13, 2024

This won't currently pass tests because PDMats needs similar improvements. But I thought this might be worth posting for comment first before going to larger efforts.

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.

Fitting does not respect type parameters
1 participant