Fix parameter type bug for non-time arrays

[corakingdon]

@jrising found this bug, more info on this forum post:
https://forum.mimiframework.org/t/array-parameter-setting-fails-with-int64-parameter-type/113

I added a line to the test file which now passes, but represents what previously would have errored.

[codecov]

Codecov Report

Merging #654 into master will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #654   +/-   ##
=======================================
  Coverage   80.22%   80.23%           
=======================================
  Files          39       39           
  Lines        2746     2742    -4     
=======================================
- Hits         2203     2200    -3     
+ Misses        543      542    -1     

Impacted Files	Coverage Δ
src/core/defcomp.jl	90.50% <0.00%> (-0.06%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 598ca04...b0f379a. Read the comment docs.

[jrising]

How does this work, if we do want to use autodifferentiation? Do things break if you set the model
number_type to Number?

The previous functionality (which admittedly was not considering all the cases) was that you could still set the parameters on a model as normal Float64 arrays, and then they would be converted internally. That's useful for models that don't always want to be run in "autodiff" mode, and don't want to duplicate all of their parameters setting code to manually convert to Number. (At least, that's my recollection of the problems that emerged.)

[corakingdon]

@jrising if the autodiff functionality that you are describing was something that was previously working for arrays that have a time dimension, then it should still work. The arrays of values still get converted to the model's default number type in the outer function call (before the function set_external_array_param! that I've modified here is invoked). set_external_array_param! is called by the original set_param!. It's in the set_param! function that the array of values provided by the user gets converted to the model's default number type, unless their is a type specified for that specific parameter (like x::Int64=Parameter(), in which case the values will stay as Int's). What I've fixed here was just a bug where only for non-time arrays, the set_external_array_param! function was unnecessarily converting all values to the model's default type again (even though they had been converted in the outer set_param! function), but it was also doing so even in the case where the user had specified a parameter-specific type (this was the bug).

Not sure if that fully answers your question though, let me know if you still think there might be a problem for specifically using "Number" as the default type!

Another thing, if you want to test it out, please don't use this branch, but try it on the param-type-0.9.6 branch. (This PR's branch is off of master, which has other breaking changes that you don't want yet, but the param-type-0.9.6 branch will only include this fix for you to use)

[corakingdon]

here's an example with the number type set to "Number" that seems to still work:

using Mimi

@defcomp bar begin
    x::Int64 = Parameter(index=[regions])
    y = Parameter(index=[regions])
    z = Parameter(index=[time])
end

m = Model(Number)
set_dimension!(m, :time, 3)
set_dimension!(m, :regions, [:a, :b])
add_comp!(m, bar)
set_param!(m, :bar, :x, [1,2])
set_param!(m, :bar, :y, [1.,2.])    # even though y and z are provided as Floats, they will be converted to Number
set_param!(m, :bar, :z, [1.,2.,3.])
run(m)
m[:bar,:x]    # returns an Array{Int64, 1}
m[:bar,:y]    # returns an Array{Number, 1}
m[:bar,:z]    # returns an Array{Number, 1}

[jrising]

Thanks for testing this! Looks like it should work smoothly.