Remaining component cleanup

[rjplevin]

I'm not sure which milestone or whom to assign these to:

1. v1.0.0 @ckingdon95 - set_param, set_external_param, set_external_{scalar,array}_param, etc. are nested in weird ways. I think one of them should be named _set_param to avoid the confusion.
2. moved to deprecate some component functions #662 (With our current "import" semantics, I think some funcs that take a ComponentPath arg may be deprecated. I doubt they're used by anyone yet.)
3. moved to deprecate some component functions #662 (Similarly, funcs taking string paths, like "/path/to/component:datum_name" are probably deprecated.)
4. moved to Fix bug in colon indexing to timestep arrays #658 (use of : in timestep array indexing)
5. v1.0.0 @ckingdon95 - I left in places instances of @defmodel in the test code where it's working. We should either commit to the macro and get it working (I can do that) or remove it and all uses thereof.
6. moved to Fix warnings from Electron in Explorer #661 (There are some errors raised in explorer tests but I haven't dug into these yet. I'd be happy to work through these with @lrennels if needed)
7. moved to Implement "joining" together of multiple parameters #657 (There is not yet support for parname = Parameter(x.p1, y.p1, z.p2), joining parameters into a single name in a component.)

Just FYI, in test_timesteparrays.jl, I left in place the first couple of uses of deprecated int indexing but revised the rest to reduce verbosity. The : issue remains, as noted above.

[davidanthoff]

@lrennels and @ckingdon95 can you copy these out into individual issues and then maybe divide them between the two of you? I assume this is all on hold until we have merged @rjplevin's branch, though, right?

[corakingdon]

7. There is not yet support for parname = Parameter(x.p1, y.p1, z.p2), joining parameters into a single name in a component.

Just checking, is there a more verbose way of accessing this functionality? For example, does the following, or something like it, work instead?

@defcomposite
    parname = Parameter()
    connect(x.p1, parname)
    connect(y.p1, parname)
    connect(z.p2, parname)
end

or is there no way of setting multiple leaf parameters to one external parameter?

The rest of your list seems like we could address later, but if this functionality doesn't work, it's the one I'd be most worried about.

[rjplevin]

No, there is no way to do it yet. I could work on this if it's critical.
Nor will the example you give work, because there's only support for a single ParameterDefReference currently, not an array of them. The simplest way to address this is probably to always store an array of them, with only 1 element in most cases.

[rjplevin]

I fear that as we've altered semantics and API, we've been collecting unused functions that should be deleted. I know of no simple way to identify these currently. In Pycharm, the "find references" features is robust and dependable -- it requires matching only name, and for methods, the first argument. Multiple method signatures in julia complicate this substantially, so I don't trust the IDEs to get it right...

[rjplevin]

Regarding the "joining" of parameters, we should determine how to handle several corner cases. Here are some with my proposed handling:

Joining parameters is permitted if:

1. the sub-component params are not otherwise connected
2. the sub-component params have no default values, or all have the same default value

Once parameters are thus joined, it should not be permitted to set any of the individual sub-comp params directly, nor to connect any of them individually. That is, you can't do:

@defcomposite foo
    Component(x)
    Component(y)
    Component(z)
    joined_param = Parameter(x.p1, y.p1, z.p2)
    connect(y.p1, z.var1) # NOT ALLOWED
end

This also suggests means disallowing set_param!(composite, ...) to be called on parameters referenced by joined parameters. If we only allow set_param!(m, ...) this is fine, but we need to remove deprecated methods, e.g., that operate on component paths or component path strings like "/foo/bar".

[corakingdon]

Joining parameters is permitted if:

1. the sub-component params are not otherwise connected
2. the sub-component params have no default values, or all have the same default value

Agreed with both of these!

This also suggests means disallowing set_param!(composite, ...) to be called on parameters referenced by joined parameters.

Yes, you wouldn't be able to call set_param! on joined parameters, because they wouldn't be bubbled up to the composite's exposed parameters anyway. I imagine it would just cause a "key not found" type error when looking for such a parameter's name in the list of the composite's external parameters.

If we only allow set_param!(m, ...) this is fine, but we need to remove deprecated methods, e.g., that operate on component paths or component path strings like "/foo/bar".

Yes, we need to disallow this, and only allow calling set_param!(m, :top_level_component, ...)

[lrennels]

@davidanthoff @ckingdon95 @rjplevin I just took a stab at assigning these, take look before I split them out into different issues ...

[lrennels]

Some of them probably could be v1.0.1 I guess