Replace multi-variable connections with functions

[kyllingstad]

Here is my current attempt at fixing #464 and providing an API for the new "functions" feature. It's not entirely done yet – see "to do" below – but since it's a rather big change, I figured I'd start getting some feedback now in case any big issues or change requests come up (but not a formal review).

Main changes

• Removes connection and related classes
• Adds function and related classes
• Adds two concrete functions, vector_sum_function and linear_transformation_function

To do

The following remains to be done before this PR is complete:

• Add support for boolean and string variables in functions
• Slightly more consistent naming
• Add tests
• Complete documentation
  Reorganise headers slightly (I'll do this as a separate pull request rather than complicate this one even further.)

What's the difference?

Basically, a function is a lot more like a simulator than a connection. You add it to an execution with execution::add_function(), and then you make (scalar) connections between simulator and function variables using execution::connect_variables(). Currently, three types of connections are allowed:

• simulator-simulator
• simulator-function
• function-simulator

Notably, function-function connections are not allowed. API- and implementation-wise it shouldn't pose too many problems, but there are some simulation-technical subtleties I'd like us to think some more about before adding it (in particular, how to deal with algebraic loops in function chains).

Like simulators/FMUs, functions also support introspection and dynamic instantiation by means of the function_type and function_type_description classes. (These correspond to model and model_description for simulators, respectively.) This will be crucial for completing the other thing I've been working on, namely #114 (system_structure).

[markaren]

This is going to be too complex for me to review again, especially since it's touches on the msmi which I'm not into at all..

However, I'm concerned that this will make the API much more difficult to use. IMO, there should still be an "easy" way of doing simple stuff like using/creating functions^* (both global and connection specific) like or similar to linear_transformation. This work raises the entry level significantly.
I mean compare the proposed way of creating/using linear_transformation with inserting a std::function somewhere in the pipeline.

*General functions, not a cse::function.

[kyllingstad]

However, I'm concerned that this will make the API much more difficult to use. IMO, there should still be an "easy" way of doing simple stuff like using/creating functions* (both global and connection specific) like or similar to linear_transformation. This work raises the entry level significantly.

I agree that the current API is easier to use, and that the proposed API will slightly increase the amount of code required for simple use cases. (Though it's worth noting that it reduces the amount of code required for the simplest use case, a direct scalar connection, making it a one-liner.)

The connection API is really nice and simple, but that is precisely because it only supports a limited set of use cases: those which can be expressed with anonymous, and hence unstructured, inputs and outputs. Examples include scalar and vector linear transformations (N anonymous inputs, N anonymous outputs) and scalar sums (N anonymous inputs, 1 anonymous output).

However, once you have cases where the inputs and outputs need to be identified and grouped, like vector sums and coordinate transformations, then the current API is no longer sufficient.

I started out trying to find some way of simply extending the connection mechanism, precisely because it works well for the use cases it currently supports, but I couldn't find a natural extension for it which didn't become more complex and cluttered than what I've proposed here.

I mean compare the proposed way of creating/using linear_transformation with inserting a std::function somewhere in the pipeline.

From the perspective of client code, I'd say it's now roughly on the same complexity level as adding and connecting a simulator to do the same job. Example:

const auto s1 = execution.add_slave(make_slave_somehow());
const auto s2 = execution.add_slave(make_slave_somehow());
const auto fn= execution.add_function(std::make_shared<linear_transformation_function>(32.0, 1.8));
execution.connect_variables({s1, variable_type::real, s1Out}, {fn, variable_type::real, fnIn});
execution.connect_variables({fn, variable_type::real, fnOut}, {s2, variable_type::real, s2In});

[ljamt]

This PR follows our discussion on handling functions as proper function structures in cse-core. I like it, and believe we're moving in the right direction.

Not able to build on windows as warnings are treated as errors:
cse/function/vector_sum.hpp(88): warning C4100: 'value': unreferenced formal parameter
warning C4267: 'argument': conversion from 'size_t' to '_Ty', possible loss of data

Not visible in CI build yet as our windows agent is down. Looking into it.

[eidekrist]

Cool beans!
The function parameterization part seems a bit complex, but I guess there is a reason for having it general.

[kyllingstad]

Thanks for the feedback so far, it's very helpful. To summarise, I will:

• Fix the concrete issues you pointed out
• Improve the documentation to explain the concepts a bit more fully
• Try to simplify (though I'm not quite sure how, and therefore can't promise anything – suggestions welcome!)

[kyllingstad]

Since nobody has commented on the proposed class and struct names, should I assume that you're all fine with them?

In particular, I'm thinking about the use of the word io to refer to function inputs and outputs. I used that instead of variable to indicate that, unlike FMU variables, function variables can only have input/output causality. (That, and the fact that function_variable_group_description would be a mouthful...) I also worried about whether it would be confusing to use parameter in the way I've done, since it's quite different from FMU variables with parameter causality. But I don't know of a better word for it.

EDIT: Also, if we're thinking of these as functions in the mathematical sense, then I suppose "inputs" and "outputs" are more appropriate terms than "variables" anyway.

[eidekrist]

Then io it is! 👍

[kyllingstad]

I have changed the order of operations in fixed_step_algorithm::initialize() and ::do_step().

[...]

Hang on, maybe this doesn't matter after all, since all functions get calculated right before the first step, by the initialize() function...

I reverted it to the old evaluation order (but kept the cleaned-up code).

[kyllingstad]

I think this is ready for review now. I've included the changes from #539 in this branch, but I've also retargeted this pull request on #539 so you don't see those changes here. Once the other PR has been merged, I'll change this one so it targets master again.

[kyllingstad]

Sorry for the last-minute changes while you are apparently in the middle of reviewing this, Levi. (The new commits don't add functionality, it's just some minor cleanup.)

[ljamt]

Nice job on introducing functions as first class citizens!
Great work on the documentation 👍

[kyllingstad]

I've retargeted this on master now, but since I unthinkingly did a squash merge rather than a normal merge of PR #539, Git no longer realises that the two branches are in fact compatible. :( So I'll need to re-fix some conflicts again.

[kyllingstad]

I've fixed the conflicts now, but I'll wait for Jenkins to come back online and check the code before merging anything.

[ljamt]

Jenkins is back up and has built this branch successfully 👍