[BUG] Inconsistent parameter types and checks when calling create_* or add_* functions vs. _add_eng_obj!, as used in parser

[John-Boik]

The function _add_eng_obj! is used in the OpenDSS to PMD parser, rather than using calls to functions such as add_transformer and add_line. This leads to several inconsistencies, several of which are discussed here. I also discussed one in Issue 463. As I suggest in that issue, if the intent is that a user should be able to create a circuit from scratch using functions like add_transformer or from parsing an OpenDSS file, and that the resulting engineering model should be the same, then it might be good to call functions like add_transformer (or create_transformer) in the parsing code, rather than _add_eng_obj!. That way, any inconsistencies are eliminated.

Issue 1: rs and xs are of type Vector if not missing in create_voltage_source, but calls to add_eng_obj! provide a Matrix. See also lines 364 and 365 in that file, for example: eng_obj["rs"] = fill(dss_obj["r_mutual"], n_conductors, n_conductors. The type of rs and xs in add_voltage_source should apparently be Union{Matrix{<:Real}, Vector{<:Real}, Missing}.

Issue 2: The lengths of pd_nom and qd_nom are checked in create_load:

n_conductors = configuration == WYE ? length(connections)-1 : length(connections)    
for v in [pd_nom, qd_nom]
    if !ismissing(v)  
        @assert length(v) == n_conductors
    end
end

This check would fail when parsing OpenDSS files for circuit IEEE123 if create_load were called. Instead, add_eng_obj! is called and the length check is never made.

A specific example is load s35a in that circuit. The Dict below is created by calling parse_file. Note that the configuration is DELTA, the size of connections is two, and the size of pd_nom and qd_nom are one. If create_load had been called with the same information given to add_eng_obj!, the length check in create_load would have failed.

Dict{String, Any} with 10 entries:
  "source_id"     => "load.s35a"
  "qd_nom"        => [20.0]
  "status"        => ENABLED
  "model"         => POWER
  "connections"   => [1, 2]
  "vm_nom"        => 4.16
  "pd_nom"        => [40.0]
  "dispatchable"  => NO
  "bus"           => "35"
  "configuration" => DELTA

Issue 2b: Similar to Issue 2, there is a size check in create_line:

n_conductors = size(f_connections)[1]
shape = (n_conductors, n_conductors)
for v in [rs, xs, g_fr, b_fr, g_to, b_to, cm_ub, sm_ub, vad_lb, vad_ub]
    if !ismissing(v)
        if isa(v, Matrix)
            @assert size(v) == shape
        else
            @assert size(v)[1] == n_conductors
        end
    end
end

This check fails when for example f_connections=[2,0] and rs is a 1x1 matrix. I'm sorry I can't locate the OpenDSS file that serves as an example. But regardless, parsing that file does not fail because add_eng_obj! is called rather than create_line and the size test is never engaged.

Issue 3: Some parameter types in create_transformer cause errors, but these do not occur when parsing OpenDSS files because _add_eng_obj! is called instead.

The parameters for create_transformer include:

tm_lb::Union{Vector{Vector{<:Real}},Missing}=missing,
tm_ub::Union{Vector{Vector{<:Real}},Missing}=missing,
tm_set::Union{Vector{Vector{<:Real}},Missing}=missing,

These should probably follow the pattern below, instead:

function fx(x::Union{Vector{Vector{T}},Missing}=missing) where {T <: Real}
    println("hello")
end

This function works when called via fx([[3.4]]). Note, however, that the following fails:

function fx(x::Union{Vector{Vector{<:Real}},Missing}=missing)
    println("hello")
end

The error is no method matching fx(::Vector{Vector{Float64}}).

Issue 4: The function create_transformer has a parameter buses, plural. The same plural parameter occurs in Line 914. But the parameter bus, singular, is used when creating a transformer via _add_eng_obj!. Also the plural term might be inconsistent with the singular term in _map_eng2math_transformer" : nrw = length(eng_obj["bus"]).

Issue 5: In create_transformer, parameters include:

vm_nom::Union{Vector{<:Real},Missing}=missing,
sm_nom::Union{Vector{<:Real},Missing}=missing,

But when parsing, _add_eng_obj! is called instead. The function _dss2eng_transformer includes:

 # kvs, kvas
    for (fr_key, to_key) in zip(["kv", "kva"], ["vm_nom", "sm_nom"])
        if isempty(dss_obj.xfmrcode) || _is_after(dss_obj.raw_dss, "$(fr_key)s", "xfmrcode") || all(_is_after(dss_obj.raw_dss, fr_key, "xfmrcode", w) for w in 1:shared["windings"])
            eng_obj[to_key] = dss_obj["$(fr_key)s"]
        else
            for w in wdgs_after_xfmrcode
                if !haskey(eng_obj, to_key)
                    eng_obj[to_key] = Vector{Union{Real,Missing}}(missing, shared["windings"])
                end
                eng_obj[to_key][w] = dss_obj["$(fr_key)s"][w]
            end
        end
    end

The line Vector{Union{Real,Missing}}(missing, shared["windings"]) provides a type for vm_nom and sm_nom that is different from the types given by create_transformer.

Issue 6: There doesn't seem to be a function add_time_series!. I believe this means that someone who is creating a circuit manually (using functions like add_line) must create the time series using _add_eng_obj!, which is a private function. Was add_time_series! commented out for some reason?