Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable getting values of all inputs #364

Closed
dhblum opened this issue Oct 6, 2021 · 7 comments
Closed

Enable getting values of all inputs #364

dhblum opened this issue Oct 6, 2021 · 7 comments

Comments

@dhblum
Copy link
Collaborator

dhblum commented Oct 6, 2021

Currently, in order be able to get the value of an input, the test case developer needs to explicitly add a read block following an overwrite block. There are some cases where this happens, e.g. in bestest_hydronic_heat_pump, and some cases where this doesn't, e.g. in multizone_residential_hydronic. In the latter, particularly look at the model ConHeaZon (figure below), which is the general implementation of the zone temperature controllers. To a user reading the measurements, they can retrieve the value of the control signal leaving the PI controller as reaActHea, but they can't retrieve the value of the controller set point leaving oveTSetHea. Thus, it is difficult to know what the baseline control is.

Screen Shot 2021-10-06 at 1 17 41 PM

My suggestion is to add functionality to the parser.py such that for every overwrite block, there is a corresponding wrapper output variable added. I believe it is reasonable to assume for a BAS that one would be able to read the value of such control signals. The form would be as follows:

  • Consider an instance of an overwrite block as ove1.
  • Add a wrapper output variable called ove1_y such that it equals ove1.y.
  • The unit, min, max, and description of ove1_y would match that of ove1_u.
  • This would add ove1_y to the list of available points using the /measurements API endpoint.

Once functionality added, I recommend removing the read blocks in the existing emulators that are used for the reading of control signals.

@SenHuang19 I know this is something that you've brought up before, but in response to #359 perhaps should move up the priority list.

Feel free to provide any feedback. FYI also @JavierArroyoBastida.

@javiarrobas
Copy link
Contributor

I think this is a necessary issue. Thanks @dhblum for bringing it up.

As for the implementation, wouldn't it be better to directly add a read block within the Overwrite Modelica class? so that parser.py does not need to be modified. I agree that the existing read blocks for reading control signals should be removed when implementing either option.

@dhblum
Copy link
Collaborator Author

dhblum commented Oct 6, 2021

True, that would relieve modification of parser.py. Although, then we need to PR Modelica IBPSA and have Buildings and IDEAS sync to propagate into test case models. Not necessarily saying this is worse.

@javiarrobas
Copy link
Contributor

I like having the process of tagging decoupled from the process of preparing the wrapped model. However, I have no strong opinion and I think both options would work fine.

@dhblum
Copy link
Collaborator Author

dhblum commented Oct 6, 2021

The process of tagging is what preparing the wrapped model is about I think though. It is actually a small edit to enable this, see 7b543ed. One benefit of this is I think in the future if we want to otherwise somehow distinguish an output that is a "current value" of an input, rather than a sensor output, through tags or changes to the API. This way does not require any more Modelica parsing to first figure out if a given read block came from within a write block or not.

@javiarrobas
Copy link
Contributor

Seems like a good enough reason to go in that direction. Also, looking at the commit it looks like parser.py doesn't need too many edits that way.

@dhblum
Copy link
Collaborator Author

dhblum commented Jan 5, 2022

Closed by #377.

@dhblum
Copy link
Collaborator Author

dhblum commented Sep 22, 2022

Note conversation of what was implemented is continued in the PR: #364. I think the documentation (see section Parsing and FMU Export) needs to be updated for what was actually implemented in that PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants