From 21dc079af957b9a946a4b2c0bd2dd0fac2e654e7 Mon Sep 17 00:00:00 2001 From: Yingbo Ma Date: Thu, 3 Oct 2024 15:05:35 -0400 Subject: [PATCH] Revert "feat: allow users to set array length via args in `@mtkmodel`" --- docs/src/basics/MTKLanguage.md | 17 ++-- src/systems/model_parsing.jl | 161 +++++++++------------------------ test/model_parsing.jl | 49 +--------- 3 files changed, 52 insertions(+), 175 deletions(-) diff --git a/docs/src/basics/MTKLanguage.md b/docs/src/basics/MTKLanguage.md index 44fe8bbc07..a2fb7d0870 100644 --- a/docs/src/basics/MTKLanguage.md +++ b/docs/src/basics/MTKLanguage.md @@ -63,14 +63,13 @@ end @structural_parameters begin f = sin N = 2 - M = 3 end begin v_var = 1.0 end @variables begin v(t) = v_var - v_array(t)[1:N, 1:M] + v_array(t)[1:2, 1:3] v_for_defaults(t) end @extend ModelB(; p1) @@ -311,10 +310,10 @@ end - `:defaults`: Dictionary of variables and default values specified in the `@defaults`. - `:extend`: The list of extended unknowns, name given to the base system, and name of the base system. - `:structural_parameters`: Dictionary of structural parameters mapped to their metadata. - - `:parameters`: Dictionary of symbolic parameters mapped to their metadata. Metadata of - the parameter arrays is, for now, omitted. - - `:variables`: Dictionary of symbolic variables mapped to their metadata. Metadata of - the variable arrays is, for now, omitted. + - `:parameters`: Dictionary of symbolic parameters mapped to their metadata. For + parameter arrays, length is added to the metadata as `:size`. + - `:variables`: Dictionary of symbolic variables mapped to their metadata. For + variable arrays, length is added to the metadata as `:size`. - `:kwargs`: Dictionary of keyword arguments mapped to their metadata. - `:independent_variable`: Independent variable, which is added while generating the Model. - `:equations`: List of equations (represented as strings). @@ -325,10 +324,10 @@ For example, the structure of `ModelC` is: julia> ModelC.structure Dict{Symbol, Any} with 10 entries: :components => Any[Union{Expr, Symbol}[:model_a, :ModelA], Union{Expr, Symbol}[:model_array_a, :ModelA, :(1:N)], Union{Expr, Symbol}[:model_array_b, :ModelA, :(1:N)]] - :variables => Dict{Symbol, Dict{Symbol, Any}}(:v=>Dict(:default=>:v_var, :type=>Real), :v_for_defaults=>Dict(:type=>Real)) + :variables => Dict{Symbol, Dict{Symbol, Any}}(:v=>Dict(:default=>:v_var, :type=>Real), :v_array=>Dict(:type=>Real, :size=>(2, 3)), :v_for_defaults=>Dict(:type=>Real)) :icon => URI("https://github.com/SciML/SciMLDocs/blob/main/docs/src/assets/logo.png") - :kwargs => Dict{Symbol, Dict}(:f => Dict(:value => :sin), :N => Dict(:value => 2), :M => Dict(:value => 3), :v => Dict{Symbol, Any}(:value => :v_var, :type => Real), :v_for_defaults => Dict{Symbol, Union{Nothing, DataType}}(:value => nothing, :type => Real), :p1 => Dict(:value => nothing)), - :structural_parameters => Dict{Symbol, Dict}(:f => Dict(:value => :sin), :N => Dict(:value => 2), :M => Dict(:value => 3)) + :kwargs => Dict{Symbol, Dict}(:f=>Dict(:value=>:sin), :N=>Dict(:value=>2), :v=>Dict{Symbol, Any}(:value=>:v_var, :type=>Real), :v_array=>Dict{Symbol, Union{Nothing, UnionAll}}(:value=>nothing, :type=>AbstractArray{Real}), :v_for_defaults=>Dict{Symbol, Union{Nothing, DataType}}(:value=>nothing, :type=>Real), :p1=>Dict(:value=>nothing)) + :structural_parameters => Dict{Symbol, Dict}(:f=>Dict(:value=>:sin), :N=>Dict(:value=>2)) :independent_variable => t :constants => Dict{Symbol, Dict}(:c=>Dict{Symbol, Any}(:value=>1, :type=>Int64, :description=>"Example constant.")) :extend => Any[[:p2, :p1], Symbol("#mtkmodel__anonymous__ModelB"), :ModelB] diff --git a/src/systems/model_parsing.jl b/src/systems/model_parsing.jl index 09ef05c172..fce2add6b7 100644 --- a/src/systems/model_parsing.jl +++ b/src/systems/model_parsing.jl @@ -180,16 +180,6 @@ function update_kwargs_and_metadata!(dict, kwargs, a, def, indices, type, var, end end -function unit_handled_variable_value(mod, y, varname) - meta = parse_metadata(mod, y) - varval = if meta isa Nothing || get(meta, VariableUnit, nothing) isa Nothing - varname - else - :($convert_units($(meta[VariableUnit]), $varname)) - end - return varval -end - function parse_variable_def!(dict, mod, arg, varclass, kwargs, where_types; def = nothing, indices::Union{Vector{UnitRange{Int}}, Nothing} = nothing, type::Type = Real, meta = Dict{DataType, Expr}()) @@ -232,66 +222,6 @@ function parse_variable_def!(dict, mod, arg, varclass, kwargs, where_types; varclass, where_types, meta) return var, def, Dict() end - Expr(:tuple, Expr(:(::), Expr(:ref, a, b...), type), y) || Expr(:tuple, Expr(:ref, a, b...), y) => begin - (@isdefined type) || (type = Real) - varname = Meta.isexpr(a, :call) ? a.args[1] : a - push!(kwargs, Expr(:kw, varname, nothing)) - varval = unit_handled_variable_value(mod, y, varname) - if varclass == :parameters - var = :($varname = $first(@parameters $a[$(b...)]::$type = ($varval, $y))) - else - var = :($varname = $first(@variables $a[$(b...)]::$type = ($varval, $y))) - end - #TODO: update `dict` aka `Model.structure` with the metadata - (:($varname...), var), nothing, Dict() - end - Expr(:(=), Expr(:(::), Expr(:ref, a, b...), type), y) || Expr(:(=), Expr(:ref, a, b...), y) => begin - (@isdefined type) || (type = Real) - varname = Meta.isexpr(a, :call) ? a.args[1] : a - if Meta.isexpr(y, :tuple) - varval = unit_handled_variable_value(mod, y, varname) - val, y = (y.args[1], y.args[2:end]) - push!(kwargs, Expr(:kw, varname, nothing)) - if varclass == :parameters - var = :($varname = $varname === nothing ? $val : $varname; - $varname = $first(@parameters $a[$(b...)]::$type = ( - $varval, $(y...)))) - else - var = :($varname = $varname === nothing ? $val : $varname; - $varname = $first(@variables $a[$(b...)]::$type = ( - $varval, $(y...)))) - end - else - push!(kwargs, Expr(:kw, varname, nothing)) - if varclass == :parameters - var = :($varname = $varname === nothing ? $y : $varname; - $varname = $first(@parameters $a[$(b...)]::$type = $varname)) - else - var = :($varname = $varname === nothing ? $y : $varname; - $varname = $first(@variables $a[$(b...)]::$type = $varname)) - end - end - #TODO: update `dict`` aka `Model.structure` with the metadata - (:($varname...), var), nothing, Dict() - end - Expr(:(::), Expr(:ref, a, b...), type) || Expr(:ref, a, b...) => begin - (@isdefined type) || (type = Real) - varname = a isa Expr && a.head == :call ? a.args[1] : a - push!(kwargs, Expr(:kw, varname, nothing)) - if varclass == :parameters - var = :($varname = $first(@parameters $a[$(b...)]::$type = $varname)) - elseif varclass == :variables - var = :($varname = $first(@variables $a[$(b...)]::$type = $varname)) - else - throw("Symbolic array with arbitrary length is not handled for $varclass. - Please open an issue with an example.") - end - dict[varclass] = get!(dict, varclass) do - Dict{Symbol, Dict{Symbol, Any}}() - end - # dict[:kwargs][varname] = dict[varclass][varname] = Dict(:size => b) - (:($varname...), var), nothing, Dict() - end Expr(:(=), a, b) => begin Base.remove_linenums!(b) def, meta = parse_default(mod, b) @@ -338,6 +268,11 @@ function parse_variable_def!(dict, mod, arg, varclass, kwargs, where_types; end return var, def, Dict() end + Expr(:ref, a, b...) => begin + indices = map(i -> UnitRange(i.args[2], i.args[end]), b) + parse_variable_def!(dict, mod, a, varclass, kwargs, where_types; + def, indices, type, meta) + end _ => error("$arg cannot be parsed") end end @@ -445,23 +380,14 @@ function parse_default(mod, a) end end -function parse_metadata(mod, a::Expr) +function parse_metadata(mod, a) MLStyle.@match a begin - Expr(:vect, b...) => Dict(parse_metadata(mod, m) for m in b) - Expr(:tuple, a, b...) => parse_metadata(mod, b) + Expr(:vect, eles...) => Dict(parse_metadata(mod, e) for e in eles) Expr(:(=), a, b) => Symbolics.option_to_metadata_type(Val(a)) => get_var(mod, b) _ => error("Cannot parse metadata $a") end end -function parse_metadata(mod, metadata::AbstractArray) - ret = Dict() - for m in metadata - merge!(ret, parse_metadata(mod, m)) - end - ret -end - function _set_var_metadata!(metadata_with_exprs, a, m, v::Expr) push!(metadata_with_exprs, m => v) a @@ -719,7 +645,6 @@ function parse_variable_arg!(exprs, vs, dict, mod, arg, varclass, kwargs, where_ end function convert_units(varunits::DynamicQuantities.Quantity, value) - value isa Nothing && return nothing DynamicQuantities.ustrip(DynamicQuantities.uconvert( DynamicQuantities.SymbolicUnits.as_quantity(varunits), value)) end @@ -731,7 +656,6 @@ function convert_units( end function convert_units(varunits::Unitful.FreeUnits, value) - value isa Nothing && return nothing Unitful.ustrip(varunits, value) end @@ -750,50 +674,47 @@ end function parse_variable_arg(dict, mod, arg, varclass, kwargs, where_types) vv, def, metadata_with_exprs = parse_variable_def!( dict, mod, arg, varclass, kwargs, where_types) - if !(vv isa Tuple) - name = getname(vv) - varexpr = if haskey(metadata_with_exprs, VariableUnit) - unit = metadata_with_exprs[VariableUnit] - quote - $name = if $name === nothing - $setdefault($vv, $def) - else - try - $setdefault($vv, $convert_units($unit, $name)) - catch e - if isa(e, $(DynamicQuantities.DimensionError)) || - isa(e, $(Unitful.DimensionError)) - error("Unable to convert units for \'" * string(:($$vv)) * "\'") - elseif isa(e, MethodError) - error("No or invalid units provided for \'" * string(:($$vv)) * - "\'") - else - rethrow(e) - end + name = getname(vv) + + varexpr = if haskey(metadata_with_exprs, VariableUnit) + unit = metadata_with_exprs[VariableUnit] + quote + $name = if $name === nothing + $setdefault($vv, $def) + else + try + $setdefault($vv, $convert_units($unit, $name)) + catch e + if isa(e, $(DynamicQuantities.DimensionError)) || + isa(e, $(Unitful.DimensionError)) + error("Unable to convert units for \'" * string(:($$vv)) * "\'") + elseif isa(e, MethodError) + error("No or invalid units provided for \'" * string(:($$vv)) * + "\'") + else + rethrow(e) end end end - else - quote - $name = if $name === nothing - $setdefault($vv, $def) - else - $setdefault($vv, $name) - end - end end - - metadata_expr = Expr(:block) - for (k, v) in metadata_with_exprs - push!(metadata_expr.args, - :($name = $wrap($set_scalar_metadata($unwrap($name), $k, $v)))) + else + quote + $name = if $name === nothing + $setdefault($vv, $def) + else + $setdefault($vv, $name) + end end + end - push!(varexpr.args, metadata_expr) - return vv isa Num ? name : :($name...), varexpr - else - return vv + metadata_expr = Expr(:block) + for (k, v) in metadata_with_exprs + push!(metadata_expr.args, + :($name = $wrap($set_scalar_metadata($unwrap($name), $k, $v)))) end + + push!(varexpr.args, metadata_expr) + return vv isa Num ? name : :($name...), varexpr end function handle_conditional_vars!( diff --git a/test/model_parsing.jl b/test/model_parsing.jl index 9d903842c2..6332dc12a5 100644 --- a/test/model_parsing.jl +++ b/test/model_parsing.jl @@ -259,8 +259,7 @@ end @test all(collect(hasmetadata.(model.l, ModelingToolkit.VariableDescription))) @test all(lastindex.([model.a2, model.b2, model.d2, model.e2, model.h2]) .== 2) - @test size(model.l) == (2, 3) - @test_broken MockModel.structure[:parameters][:l][:size] == (2, 3) + @test size(model.l) == MockModel.structure[:parameters][:l][:size] == (2, 3) model = complete(model) @test getdefault(model.cval) == 1 @@ -314,6 +313,7 @@ end @test_throws TypeError TypeModel(; name = :throws, par3 = true) @test_throws TypeError TypeModel(; name = :throws, par4 = true) # par7 should be an AbstractArray of BigFloat. + @test_throws MethodError TypeModel(; name = :throws, par7 = rand(Int, 3, 3)) # Test that array types are correctly added. @named type_model2 = TypeModel(; par5 = rand(BigFloat, 3)) @@ -474,8 +474,7 @@ using ModelingToolkit: getdefault, scalarize @named model_with_component_array = ModelWithComponentArray() - @test_broken eval(ModelWithComponentArray.structure[:parameters][:r][:unit]) == - eval(u"Ω") + @test eval(ModelWithComponentArray.structure[:parameters][:r][:unit]) == eval(u"Ω") @test lastindex(parameters(model_with_component_array)) == 3 # Test the constant `k`. Manually k's value should be kept in sync here @@ -877,45 +876,3 @@ end end), false) end - -@testset "Array Length as an Input" begin - @mtkmodel VaryingLengthArray begin - @structural_parameters begin - N - M - end - @parameters begin - p1[1:N] - p2[1:N, 1:M] - end - @variables begin - v1(t)[1:N] - v2(t)[1:N, 1:M] - end - end - - @named model = VaryingLengthArray(N = 2, M = 3) - @test length(model.p1) == 2 - @test size(model.p2) == (2, 3) - @test length(model.v1) == 2 - @test size(model.v2) == (2, 3) - - @mtkmodel WithMetadata begin - @structural_parameters begin - N - end - @parameters begin - p_only_default[1:N] = 101 - p_only_metadata[1:N], [description = "this only has metadata"] - p_both_default_and_metadata[1:N] = 102, - [description = "this has both default value and metadata"] - end - end - - @named with_metadata = WithMetadata(N = 10) - @test getdefault(with_metadata.p_only_default) == 101 - @test getdescription(with_metadata.p_only_metadata) == "this only has metadata" - @test getdefault(with_metadata.p_both_default_and_metadata) == 102 - @test getdescription(with_metadata.p_both_default_and_metadata) == - "this has both default value and metadata" -end