Delayed feedback expressions - [merged] #1260

Closed SGeeversAtVortech closed 1 week ago

In GitLab by @baayen on May 17, 2018, 18:01

Merges delayed-feedback-expressions -> master

To Do:

[x] Compute and prepend history for delayed expression.

In GitLab by @baayen on May 18, 2018, 24:22

added 1 commit

a1ca85a5 - Compute history for delay expressions.

Compare with previous version

In GitLab by @baayen on May 18, 2018, 24:22

marked the checklist item Compute and prepend history for delayed expression. as completed

In GitLab by @baayen on May 18, 2018, 24:23

unmarked as a Work In Progress

In GitLab by @jvande42b on May 18, 2018, 21:42

added 20 commits

a1ca85a5...ae399329 - 16 commits from branch master
54db74cf - Work on delayed feedback expressions as now supported by pymoca. Not finished.
142f55d5 - Port delayed_feedback implementation to new pymoca API
5470188f - Compute history for delay expressions.
de21962a - don't call delayed_feedback_function if there are no delayed feedback variables…

Compare with previous version

In GitLab by @jvande42b on May 18, 2018, 21:44

marked as a Work In Progress

In GitLab by @jvande42b on May 18, 2018, 22:08

added 1 commit

464edd8b - make delay formualtion work when model is cached too

Compare with previous version

In GitLab by @jvande42b on May 19, 2018, 02:03

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1034

I imagine np.unique() could really clean this line up.

In GitLab by @jvande42b on May 19, 2018, 05:51

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1034

changed this line in version 6 of the diff

In GitLab by @jvande42b on May 19, 2018, 05:51

added 1 commit

8dbc0931 - fix brocken logic with np.unique(), and add comments

Compare with previous version

In GitLab by @jvande42b on May 19, 2018, 06:22

@vreeken I got a model with a delay element to work. Its rudimentary, and has issues with defining the initial states. Probably need to rework the logic on this.

In GitLab by @baayen on May 19, 2018, 17:32

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1057

Right. This needs to use the direct output of OptimizationProblem.constant_inputs() instead of the pre-interpolated dictionary. Clearly, we need a unit test here.

In GitLab by @baayen on May 19, 2018, 17:33

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1037

That is much nicer indeed!

By convention (and code in case of CSVMixin) the last item in the history time series is the t0 value, hence the [:-1].

In GitLab by @jvande42b on May 20, 2018, 14:26

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1037

ok, good to know. I will put in an assertion just to avoid confusing errors, in case the convention is not followed.

In GitLab by @jvande42b on May 20, 2018, 14:36

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1057

Maybe we could just store the interpolated constant_input values as interpolated_constant_inputs instead of overwriting the constant_inputs variable.

In GitLab by @jvande42b on May 20, 2018, 14:41

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1057

or the give the direct output of constant_inputs() the name raw_constant_inputs. I think this less confusing than changing the meaning of the variable halfway though the method.

In GitLab by @jvande42b on May 21, 2018, 19:47

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1057

changed this line in version 7 of the diff

In GitLab by @jvande42b on May 21, 2018, 19:47

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1037

changed this line in version 7 of the diff

In GitLab by @jvande42b on May 21, 2018, 19:47

added 1 commit

e786fa83 - rename the non-interpolated constant_inputs to raw_constant_inputs, improve comments

Compare with previous version

In GitLab by @jvande42b on May 21, 2018, 19:49

resolved all discussions

In GitLab by @vreeken on May 21, 2018, 20:02

Commented on src/rtctools/optimization/modelica_mixin.py line 185

Does this also work with inputs (and not just parameters)? We need that to work for the homotopy/convexity fix.

In GitLab by @vreeken on May 21, 2018, 20:12

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1033

I don't think we use this type of comment formatting anywhere else, so I'd prefer the original comment.

In GitLab by @jvande42b on May 21, 2018, 20:13

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1033

changed this line in version 8 of the diff

In GitLab by @jvande42b on May 21, 2018, 20:13

added 1 commit

429bc4aa - flake8 fixes

Compare with previous version

In GitLab by @jvande42b on May 21, 2018, 20:19

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1085

Instead of time, @baayen had a value of 0.0 here. I imagine this was on accident, otherwise there would be no need for enumerate().

In GitLab by @jvande42b on May 21, 2018, 20:29

Commented on src/rtctools/optimization/modelica_mixin.py line 185

That is next.

In GitLab by @baayen on May 21, 2018, 23:24

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1085

Good catch!

In GitLab by @jvande42b on May 23, 2018, 04:59

Commented on src/rtctools/optimization/modelica_mixin.py line 185

changed this line in version 9 of the diff

In GitLab by @jvande42b on May 23, 2018, 04:59

added 1 commit

a37f55da - resolve delay expressions that contain constant inputs

Compare with previous version

In GitLab by @jvande42b on May 24, 2018, 06:49

added 2 commits

610a1a22 - add assertion and minor refactor
8c216495 - assert that delay expressions do not depend on states, and remove lingering…

Compare with previous version

In GitLab by @jvande42b on May 24, 2018, 07:16

added 1 commit

fecb2613 - assert that delay expressions do not depend on states, and remove lingering…

Compare with previous version

In GitLab by @baayen on May 24, 2018, 08:04

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1059

Using numpy here rather than casadi is probably more efficient. All historical values are numerical.

In GitLab by @baayen on May 24, 2018, 08:06

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1130

Do we need the above call?

Either delay depends on symbol_list, in which we case we abort with an assertion failure.

Or it does not, but then we don't need the substitution.

In GitLab by @baayen on May 24, 2018, 08:08

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1141

in_times -> collocation_times

(Later on, we do collocation_times - delay, so the array lengths should match up)

In GitLab by @vreeken on May 24, 2018, 12:11

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1130

I haven't checked myself in this particular case, but according to Jesse we do have to do a substitution (either here or in Pymoca) for cached models. CasADi thinks there's a dependency on states etc., because the delay_function was defined that way and called that way (with MX symbols, so lazy evalulation). depends_on is generally quite an accurate check, but anything else (sparsity, is_zero, ...) cannot be expected to give the right answer, including apparently numerical function calls with not all arguments specified.

I think the solution/workaround is to either pass NaNs in when you want to get a numeric value, or use .expand() (before .map()) on the function 'delay_values' below for (more accurate, but possibly still wrong) numeric values/sparsity/is_zero stuff. Fun stuff.

I do feel (as I told Jesse) that these checks/substitutions would belong better in Pymoca, but they will be quite similar (almost copy-paste).

In GitLab by @jvande42b on May 24, 2018, 15:37

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1130

I agree with moving them to pymoca. I will move it over- just wanted to get it working last night. When done in pymoca, the delay will behave the same with cached and unchached models, even if the internal structure is different.

In GitLab by @jvande42b on May 24, 2018, 15:38

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1141

good point.

In GitLab by @jvande42b on May 25, 2018, 05:04

Not sure why gitlab is not showing my latest commit on this MR...

In GitLab by @jvande42b on May 25, 2018, 05:06

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1130

Fix is now here: https://github.com/pymoca/pymoca/pull/118/commits/2fc8607d9b0c7466ceef9c4a7156da721d374ca7

In GitLab by @jvande42b on May 25, 2018, 05:37

Commented on src/rtctools/optimization/collocated_integrated_optimization_problem.py line 1130

changed this line in version 13 of the diff

In GitLab by @jvande42b on May 25, 2018, 05:37