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

Array function registration doesn't overload promote types correctly #1128

Closed
YingboMa opened this issue May 3, 2024 · 3 comments · Fixed by #1129
Closed

Array function registration doesn't overload promote types correctly #1128

YingboMa opened this issue May 3, 2024 · 3 comments · Fixed by #1129

Comments

@YingboMa
Copy link
Member

YingboMa commented May 3, 2024

MWE:

using Symbolics
@register_array_symbolic bar(x::AbstractVector, p::AbstractMatrix) begin
    size = size(x)
    eltype = promote_type(eltype(x), eltype(p))
end
@variables x[1:3] p[1:3, 1:3];
ex = bar(x, p)
ex2 = similarterm(Symbolics.unwrap(ex), bar, [Symbolics.unwrap(x), Symbolics.unwrap(p)]; metadata=Symbolics.metadata(ex))
typeof(ex2)

outputs

julia> typeof(ex2)
SymbolicUtils.BasicSymbolic{Any}
YingboMa added a commit to SciML/ModelingToolkit.jl that referenced this issue May 3, 2024
@ChrisRackauckas
Copy link
Member

@AayushSabharwal can you take a look at this?

@AayushSabharwal
Copy link
Contributor

It looks like @register_array_symbolic doesn't overload promote types at all, which is not ideal

@AayushSabharwal
Copy link
Contributor

promote_symtype is difficult. The best that seems to be possible is container_type{etype} where container_type is either specified in the macrocall or defaults to promote_atype(f, args...) and etype is the eltype in the macrocall or promote_eltype(f, args...). The dimensionality cannot be generically inferred, since promote_symtype only gets the types of arguments as input so it can't run the size provided in the macrocall, and relying in promote_ndims would break every existing @register_array_symbolic since it errors by default.

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 a pull request may close this issue.

3 participants