Time dependent modifiers

[eidekrist]

This PR is a different take on PR #463, aiming to solve #203. The difference this time is that the API now supports modifier functions which depend on time information, and that these only need to be set once.

Premise:

• I want to set a modifier on an unconnected input variable. The modifier should add a bias to the variable value, which is gradually ramped up with each step.
• I want to specify the slope of the ramp with unit per second - such that a slope value of 2.0 increases the variable value with 2.0 over the duration of one second.

In order to achieve this, two changes are necessary:

• To apply the slope value correctly, the modifier function needs access to the current step size, since I can't be sure which step size the algorithm will use.
• In order to gradually increase the modified value with each step, the modifier function must be run on each step, regardless if a value has been set from the outside.

Discussion needed: The last point breaks the "simulation correctness" principle discussed earlier when connecting a slow simulator to a faster one (but only if setting a modifier function). Then again, if you are modifying a variable, simulation correctness is probably not your top priority.

The modifier functions now take the form of

std::function<value_type(value_type, cse::duration)>

where cse::duration represents the step size for "this step". I tried finding a reason/use case for also including the current simulation time, but was unsuccessful, so I left it out.

[kyllingstad]

Discussion needed: The last point breaks the "simulation correctness" principle discussed earlier when connecting a slow simulator to a faster one (but only if setting a modifier function). Then again, if you are modifying a variable, simulation correctness is probably not your top priority.

Not sure I understood this. Can you elaborate and/or give an example of what you mean?

[kyllingstad]

I tried finding a reason/use case for also including the current simulation time, but was unsuccessful, so I left it out.

If the functions only receive step sizes, any function that depends on absolute time would have to maintain an accumulated sum of time steps. This makes it a bit impractical to use lambdas for such cases.

But let's keep it as-is for now and see how it works out. It's just a matter of adding an additional argument anyway.

[eidekrist]

Not sure I understood this. Can you elaborate and/or give an example of what you mean?

Example:

• Sim A has step size decimation factor 2.
• Sim B has step size decimation factor 1.
• Sim A's output is connected to Sim B's input.
• In the normal case, values will be transferred from Sim A to Sim B every other base step, and we make sure Sim B -> setReal is only called on the steps where values are transferred through the connection. This is the "correctness" thing - Sim B might extrapolate its inputs since they were not set.
• With the proposed changes, if setting a modifier function on Sim B's input, Sim B -> setReal will be called every single time Sim B is stepped.

[kyllingstad]

Very nice and clean solution!

[kyllingstad]

• In the normal case, values will be transferred from Sim A to Sim B every other base step, and we make sure Sim B -> setReal is only called on the steps where values are transferred through the connection. This is the "correctness" thing - Sim B might extrapolate its inputs since they were not set.
• With the proposed changes, if setting a modifier function on Sim B's input, Sim B -> setReal will be called every single time Sim B is stepped.

Ok, I see now. Then I agree with you. This is something users will simply have to understand and accept if they use modifiers.

[kyllingstad]

Hang on, don't merge quite yet. I had one review comment that seems to have been lost for some reason. Let me write it again so you can consider it before merging.

OK, done. :)

[kyllingstad]

You don't need the accumulator pointer, you can just use acc_ directly inside the lambda, because this is implicitly captured when you use [=].

Alternatively, if you want to avoid having to refer to an outside variable, you can add it to the lambda directly:

[=, acc = 0.0](double original, cse::duration deltaT) mutable { /* use and modify acc here */ }

If you keep your solution or use the former of mine, I suggest that you make the class noncopyable and nonmovable. Otherwise, the lambda will have a pointer to the class instance, and if the instance inadvertently gets moved to a different memory location, you're in trouble. (For a simple test this is no big risk, but it's a good habit to always do this when you leak this pointers to lambdas.)

[eidekrist]

Great stuff, thanks! I learnt a lot from that. According to the fountain of wisdom making the class noncopyable also makes it nonmovable.

[kyllingstad]

Lots of good advice in that SO answer that I wasn't aware of, thanks! Time to go undelete all of my deleted move constructors, brb. 😄

[ljamt]

Clean solution and well documented with the test 👍