Pipline MHE #2

kuanhanl · 2022-05-01T19:56:38Z

Add an MHE example for the pipeline, including a test for it.

Robbybp · 2022-05-02T00:52:21Z

Thanks Kuan-Han, this looks really good at first glance. I especially like the small size of the functions in the MHE constructor file. I intend to give this a detailed review relatively soon.

kuanhanl · 2022-05-02T15:10:41Z

Thanks Kuan-Han, this looks really good at first glance. I especially like the small size of the functions in the MHE constructor file. I intend to give this a detailed review relatively soon.

You're welcome, Robby! I really enjoyed reviewing your codes and learning from them. Please let me know if you have any questions and comments.

… as json

Robbybp

Hi Kuan-Han, I think I will review this code in three stages:

Make sure my data structures and functions are holding up to the task of using them for MHE. It seems like you are using the linker, helper, and time series data structures effectively, but I noticed that you could be using my functions for creating "cost expressions." I implemented this in a separate branch, kuanhanl#2, if you are curious.
Add functionality to my data structures and functions that may be useful based on your code. This might be something like a class for "scalar data from a time-indexed variable" for things like objective weights. I'll use your MHE as a test case for this and open another PR into this branch when I have this functionality.
Review the actual implementation of MHE here, e.g. the APIs to construct model components.

My motivation for these separate stages in this order is that I can do the first two independently (other than merging PRs into your MHE branch, which is not strictly necessary for me to work on these). Once these first two have been done and tested with both NMPC and MHE (although not necessarily merged into this branch), I can go ahead and work on a PR into IDAES with the data structures and utility functions. Then, when you have time to look at this again, you can make comments, review and merge my PRs, and address whatever comments I might have on the MHE API when I get around to it.

Robbybp · 2022-06-01T19:14:09Z

nmpc_examples/pipeline/control_input_data.json

@@ -0,0 +1 @@
+{"fs.pipeline.control_volume.flow_mass[*,1.0]": [553003.1393240632, 535472.4292275416], "fs.pipeline.control_volume.pressure[*,0]": [58.905880452751845, 58.92036532907178]}


Where does this come from? I never like having a "data file" in a repository without the script to generate it. You can see the get_control_inputs function in my PR, which creates a TimeSeriesData object with this info. This can be serialized if necessary with json.dump and TimeSeriesData.to_serializable.

These control inputs come from the results of run_pipeline_nmpc.py.
Yes, I think using TimeSeriesData to wrap these control inputs is more appropriate and easier to understand for users.

Robbybp · 2022-06-01T19:14:42Z

nmpc_examples/pipeline/run_pipeline_mhe.py

+    #
+    # Load data into dynamic model
+    #
+    use_linker = False


Code fails if use_linker is set to True.

Robbybp · 2022-06-01T19:15:21Z

nmpc_examples/pipeline/run_pipeline_mhe.py

+    else:
+        # If I want to use DynamicModelHelper:
+        m_plant_helper = DynamicModelHelper(m_plant, m_plant.fs.time)
+        m_plant_helper.load_scalar_data(scalar_data)
+        m_plant_helper.load_data_at_time(initial_data)


Failure with use_linker was for two reasons:

The DynamicModelHelper is used elsewhere

The plant simulation doesn't converge if scalar data is not initialized

Yes, you are right.
I saw you construct DynamicModelHelper for plant and initialize the scalar data before if statement.
Thanks for fixing this!

Robbybp · 2022-06-01T19:23:33Z

nmpc_examples/mhe/mhe_constructor.py

+    #TODO: This function is similar to "get_tracking_cost_from_constant_setpoint",
+    #Should I merge this one to that one?


As far as I can tell, this function is basically just doing what get_tracking_cost_from_constant_setpoint does, except it uses zero as the setpoint value for every variable, and specifies the weights in terms of "components" rather than the variables that appear in the cost expression. My approach to merge the two functions has been to just call get_tracking_cost... with a setpoint data structure with all zeros, then to require that weights are specified in terms of error variables. I don't think it should be this function's responsibility to assign the correct weight to the correct error variable. That should already be done. You can see my approach in my PR: kuanhanl#2.

Cool, I should have thought of setting the setpoint data to zeros.
Just a minor suggestion, shall we remove "Tracking" in line 69 of nmpc_examples/nmpc/cost_expressions.py because MHE is using this function as well?

I also see you have a toggle between two different objective formulations for constructing measurement error costs. It looks good.

Robbybp · 2022-06-01T19:24:53Z

nmpc_examples/pipeline/tests/test_pipeline_mhe.py

+    def test_small_mhe_simulation(self):
+        simulation_data, estimate_data = run_mhe(
+            simulation_horizon=4.0,
+            sample_period=2.0,
+        )


This test takes quite a while with the default estimator horizon (20 h, I think), so I shortened it. I end up getting the same answers either way. Does this make sense?

Yes, it does make sense in this case and this is related to your side question.

Robbybp · 2022-06-01T19:27:56Z

Side question: For MHE, should we be adding noise to the measurements? Also, in the example provided, our optimization problems converge with objective function values of zero. Is this expected, or a typical case?

kuanhanl · 2022-08-29T17:20:43Z

@Robbybp To answer your side question.
In this example, you can see I directly transfer some outputs from plant to estimator as measurements without adding any noise. Therefore, it's reasonable to expect the optimization problems converge with objective values of zero and the estimation accuracy is the same even if you shorten the horizon. This is because the optimizer should be able to find the exact true states based on the true controls, noise-free measurements and the dynamic model.

For a typical case or the real-world application, yes, we should add noise to the measurements because measurements are seldom noise-free.

DNMY - patch to MHE with new data structure

Use existing functions and data structures wherever possible in MHE

kuanhanl added 9 commits April 27, 2022 21:15

construct mhe for pipeline

2c632d2

Merge remote-tracking branch 'origin/main' into pipline-mhe

0ae7143

add function descriptions

61f3ff5

add description for the function of adding cost expression

1edaf7e

delete unused codes

f11bd44

fix a bug

79ce495

redefine time points for control inputs

5c29057

import control data with os module

4c4519b

add test for pipeline mhe

16ba745

kuanhanl changed the title ~~Pipline mhe~~ Pipline MHE May 1, 2022

kuanhanl added 2 commits May 2, 2022 11:05

minor changes

3c4c01c

fix variable bugs

6894c66

Robbybp added 10 commits May 31, 2022 16:12

adjust test to use a smaller estimator model and therefore run faster

465ae92

construct control_input_data in a python function rather than leaving…

f9b0529

… as json

comment about getting json file if necessary

c379a63

comment on disturbance cost function

c80f122

make sure code work swith use_linker=True

12b399a

update log

f019410

add small get_error_cost function

34d3544

remove get_error_disturbance_cost

6ae4872

update to use new get_error_cost function

f993c77

update log

69c0e27

Robbybp reviewed Jun 1, 2022

View reviewed changes

Robbybp added 4 commits July 25, 2022 17:15

Merge branch 'scalar-data' into mhe-patch-scalar

2d9887e

fix name of flag

4463b31

Merge branch 'mhe-patch' into mhe-patch-scalar

aeede4b

remove comments and simplify slightly

2f7c36b

Robbybp added 3 commits July 25, 2022 17:54

remove old commented code

a689f66

comment on confusing name

0024914

update log

4ae5ab7

Robbybp and others added 5 commits August 31, 2022 16:10

Merge branch 'main' into mhe-patch

2b9238a

Merge branch 'mhe-patch' into mhe-patch-scalar

c9aded1

remove additions from git merge

76825fa

Merge pull request Robbybp#3 from Robbybp/mhe-patch-scalar

c985f9a

DNMY - patch to MHE with new data structure

Merge pull request #2 from Robbybp/mhe-patch

c6b763e

Use existing functions and data structures wherever possible in MHE

		@@ -0,0 +1 @@
		{"fs.pipeline.control_volume.flow_mass[,1.0]": [553003.1393240632, 535472.4292275416], "fs.pipeline.control_volume.pressure[,0]": [58.905880452751845, 58.92036532907178]} No newline at end of file

		#TODO: This function is similar to "get_tracking_cost_from_constant_setpoint",
		#Should I merge this one to that one?

Pipline MHE #2

Are you sure you want to change the base?

Pipline MHE #2

Uh oh!

Conversation

kuanhanl commented May 1, 2022

Uh oh!

Robbybp commented May 2, 2022

Uh oh!

kuanhanl commented May 2, 2022

Uh oh!

Robbybp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Robbybp commented Jun 1, 2022

Uh oh!

kuanhanl commented Aug 29, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants