Skip to content

Avoid that inverse(::VectorTransform, x) returns a Vector{Union{}}? #132

@devmotion

Description

@devmotion

In generic code, it's very convenient to use inverse(::VectorTransform, x) (or in my case mainly inverse(::TransformTuple, x::NamedTuple)). In my application we also allow users to fix certain parameters during inference by internally replacing their transforms with Constant transforms. This works nicely.

However, users may actually fix all population parameters (and only perform inference of individual parameters), which implies that all transforms in the TransformTuple of population parameters are Constants. Unfortunately, in this case inverse with the TransformTuple returns a Vector{Union{}} instead of e.g. a Vector{Float64} or Vector{Int}, but support for Vector{Union{}} in downstream applications is much more limited, compare e.g.

julia> sum(Float64[])
0.0

julia> sum(Union{}[])
ERROR: ArgumentError: reducing over an empty collection is not allowed; consider supplying `init` to the reducer
...

julia> mean(Float64[])
NaN

julia> mean(Union{}[])
ERROR: ArgumentError: reducing over an empty collection is not allowed; consider supplying `init` to the reducer
...

julia> std(Float64[])
NaN

julia> std(Union{}[])
ERROR: ArgumentError: reducing over an empty collection is not allowed; consider supplying `init` to the reducer
...

Therefore, I switched to a custom

function _no_bottom_inverse(trf::TransformVariables.VectorTransform, x)
  T = TransformVariables.inverse_eltype(trf, x)
  if T === Union{}
    T = Float64
  end
  return TransformVariables.inverse!(
    Vector{T}(undef, TransformVariables.dimension(trf)),
    trf,
    x,
  )
end

but I'm somewhat unhappy because a) it requires that all calls of inverse are replaced with _no_bottom_inverse (which seems easy to break in future PRs) and b) it seems like a problem that might be useful to solve upstream.

IMO an array with non-numerical element type is never desirable, so maybe the implementation of inverse(::VectorTransform, x) should already handle this case, similar to _no_bottom_inverse? Maybe one should go even further and make it return float(T)?
I also wonder if it should be handled on the level of inverse_eltype(::VectorTransform, x) (which was not really an option in my downstream code).

(IMO inverse_eltype(::Constant) should not be changed - I think Union{} is the right choice there, it avoids unnecessary promotions which might be relevant for e.g. operations with Float32.)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions