1492 window ventilation

[Jun-Jiang-92]

Add the package "WindowVentilation" under "AixLib.Airflow". This package provides several empirical expressions based on the literature to estimate the air volume flow by single-sided window opening.

Link to issue #1492.

[FWuellhorst]

@Jun-Jiang-92 Thanks for the big new package! I would suggest to split up the review, so that others get customed to review, as well. @MZuschlag, are you interested in the review of this?

[Jun-Jiang-92]

@FWuellhorst I renewed (almost) all the code in this new package to meet the rules of namespace. At the same time, some models were reconstructed and debugged for a more stable performance.
The pipeline in git-CI doesn't work at the moment. I don't know why.
Would you be so kind as to review my code? For questions I'm available with pleasure.

[FWuellhorst]

@Jun-Jiang-92 : I will update this branch with the latest branch in #1500, as here the CI works with regard to HTML errors and simulation checks.

[FWuellhorst]

@Jun-Jiang-92 The model looks really nice, thanks for your work!
Beside the inline comments, I have some general requests:

1. The examples produce warnings, which should not be the case. The error messages look more like "debug" messages, which may confuse users. Is this message really necessary?

Warning: The following was detected at time: 0
  The polynomial under the square root to calculate V_flow_th is less than 0, the V_flow_th will be set to 0
  Failed condition: warrenParkins.intRes > 1E-15

2. You have a lot of assertions, which are fine. But you can move the ones which depend on parameters to "initial equation" or "initial algorithm" section to indicate those are design assertations not triggered during simulation.
3. You use some if else with >=, e.g. on the incidence angle AixLib.Airflow.WindowVentilation.EmpiricalExpressions.LarsenHeiselberg. If users control the window opening based on the value and set the set value of the control to e.g. 105 in this case, you will get a lot of state events. So double check if greater equal comparisons are "robust" in the way that the variables are not going to be controlled. Else, don't use the "equal" part or even better, a small hysteresis.
4. Can you add "simulate and plot" scripts for your examples? This way, you ensure the results won't change in future commits or at least you get an error and can check what did change.
5. Increase the diagram size in the examples to fit all model instances

[Jun-Jiang-92]

@FWuellhorst Thank you for your review! I already pushed the revised version. Regarding your questions:

1. The examples produce warnings, which should not be the case. The error messages look more like "debug" messages, which may confuse users. Is this message really necessary?

They are not debug-msgs but indeed necessary warning-msgs. A lot of empirical models in the literature do not consider the sign of polynomial under the square root, because this polynomial is always positive with their measurment data (also used as training data), which are conducted with limited boundary conditions that did not cover all the temperauture and wind ranges. By negative polynomial, I replace it to 0, in order to keep the simulation running.
Therefore, these empirical models will become inaccurate if the polynomial is negative, in which case the warning-msg is important.
The examples produce a lot of warnings, because I set a wide range of inputs there. This makes these examples like a "stress test". But in practice, the warning will be rarely triggerd. I already test these models with some field test data from the facade test bench, there was only few warnings.

2. You have a lot of assertions, which are fine. But you can move the ones which depend on parameters to "initial equation" or "initial algorithm" section to indicate those are design assertations not triggered during simulation.

Good point. I already moved assertations depended on params to "initial equation".

3. You use some if else with `>=`, e.g. on the incidence angle `AixLib.Airflow.WindowVentilation.EmpiricalExpressions.LarsenHeiselberg`. If users control the window opening based on the value and set the set value of the control to e.g. 105 in this case, you will get a lot of state events. So double check if greater equal comparisons are "robust" in the way that the variables are not going to be controlled. Else, don't use the "equal" part or even better, a small hysteresis.

I changed '>=' to '>'. The incidence angle is the difference of wind direction (from weather data) and the azimut of window / facade (parameter, fixed). The zero crossing will happen by intepolating the weather data. So, there will not be lots of event. The most other comparisons, specially comparing the "polynomial under square root" with 0, are all set with NoEvent to avoid zero crossing.

4. Can you add "simulate and plot" scripts for your examples? This way, you ensure the results won't change in future commits or at least you get an error and can check what did change.

I added them and hope they work well.

5. Increase the diagram size in the examples to fit all model instances

Done.

If you have any questions, feel free to ge in touch with me.

[FWuellhorst]

@Jun-Jiang-92 , ok, regarding the example with the warnings: Can you modify the input range so that the warnings occurs only once and document this in the example itself?

[Jun-Jiang-92]

@FWuellhorst It's impossible to adjust the range to trigger warning only once for each model. So, I changed the ranges so that no warnings will be triggered.

[FWuellhorst]

ah ok, because you have multiple instances of the model? Well, you could add an assertion counter and only trigger the warning once. Also, the warnings may be used in the partial model as far as I can see. I will implement an example and you can revert if you don't like the change.

[FWuellhorst]

I changed all models except your own expression, as this extends AixLib.Airflow.WindowVentilation.BaseClasses.PartialEmpiricalFlow. Please check if usage of extends is possible in this case as well.
Also, the simulate and plot scripts fail in ci:

*** Error: Failed to find Tolerance in '/builds/EBC/EBC_all/github_ci/AixLib/AixLib/../AixLib/Airflow/WindowVentilation/Examples/OpeningArea.mo'.
*** Error: Failed to find Tolerance in '/builds/EBC/EBC_all/github_ci/AixLib/AixLib/../AixLib/Airflow/WindowVentilation/Examples/VentilationFlowRateSimpleOpening.mo'.
*** Error: Failed to find Tolerance in '/builds/EBC/EBC_all/github_ci/AixLib/AixLib/../AixLib/Airflow/WindowVentilation/Examples/VentilationFlowRateSashOpening.mo'.```
Can you add the Tolerance in the experiment setup in the example?

[Jun-Jiang-92]

@FWuellhorst I rebuilt the assertion check into AixLib/Airflow/WindowVentilation/BaseClasses/PartialEmpiricalFlow.mo, so that every ventilation expression uses this check. But somehow, the warning triggered by t=0 will not be counted by the counter. So for now, the first two warnings will be shown in the simulation examples. Do you know how to fix it?

[FWuellhorst]

@Jun-Jiang-92 My last comments, as I had a closer look at the examples to implement the assertion:

1. You connect inputs with inputs. This is not recommend, as it's more complex to analyze where an input originates. Also, if you delete a block with an unused output, you would not expect missing inputs in other instances.
2. The simulation of 180 s generates a whooping 333 state events. Using simulation analysis, the main driver is larsenHeiselberg (ca. 200 events), then caciolo (ca. 80 events) and then vdi2078_1 (18 events). Removing these three options yields only 12 events (which are still many for 180s. The remainder of events are for maas (ca. 8), tang (2) and jiang (one) at weird times (see picture).
   Coupled to a high-order model with multiple windows and zones, this behavior will drastically increase simulation time of the already slow HOM. I am not sure if you can avoid the events or if the approaches are not interesting for you, but in any case a small hint in the doc of these models would be fair for users so they know what causes the slow simulation.
   You can check the "Simulation Analysis" option in Dymola to analyze the state events. (time events are not that big of a problem)
   

[Jun-Jiang-92]

@FWuellhorst I reconnected all of the inputs directly to sources. Thank you for reminding.

Regarding the huge amount of events, they are because I set the examples again into 'stress test' with large ranges of input conditions (see the first explanation in #1495 (comment)). In practice use, this will not happen.

I already added more DocStr into examples to explain it. If you like, I can revert the boudary condition settings back to f170771 that no warnings will be triggered.

[FWuellhorst]

Not necessary, it looks good! Thank you for this big work, they seem like really cool models :) Let's wait approx 30 min to see if the CI still passes, which it looks like. Then you can merge.