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

Reference file for model Buildings.DHC.Loads.BaseClasses.Examples.CouplingRCZ1Valve does not match with Dymola 2024 #3881

Open
AndreaBartolini opened this issue Jun 12, 2024 · 10 comments

Comments

@AndreaBartolini
Copy link
Contributor

the reference file used by the OpenModelica CI to check the temperature bui.disFloCoo.senTSup.T of the model Buildings.DHC.Loads.BaseClasses.Examples.CouplingRCZ1Valve has the following trend:
image

The same model simulated by using Dymola Version 2024x, 2023-10-06 on Windows 11 pro - 64 bit generates the following trend for the same temperature:
image

By the way, the trend generated by Dymola 2024x on Win 11 matches the same generated by OpenModelica v1.24.0-dev-121-gd21af17721 (64-bit), as shown below:
image

Can you please check if your official reference file is according to the one generated by Dymola 2024x?

Thanks in advance.

Keep @casella in the loop

@JayHuLBL
Copy link
Contributor

JayHuLBL commented Jun 12, 2024

@AndreaBartolini

I run the same model have following results:

Dymola 2024x, on Windows 10:
Screenshot 2024-06-12 at 10 01 43 AM

Dymola 2024x and Dymola 2024xRefresh1, on Ubuntu 22.04, and Dymola 2023x on Ubuntu 20.04
Screenshot 2024-06-12 at 11 05 03 AM

OpenModelica version 1.24.0~dev-30-gdbec042, on Ubuntu 22.04:

re

It shows that the OS difference will cause the results quite different.

However, the reference results should be generated by Dymola 2024x, on Ubuntu 20.04.

@casella
Copy link
Contributor

casella commented Jun 13, 2024

Is that temperature well-defined, in physical terms? Otherwise, the result would depend on numerical implementation details.

@AndreaBartolini
Copy link
Contributor Author

AndreaBartolini commented Jun 13, 2024

The temperature is from the following sensor:
image

And herebelow is reported the trend of the flow through the sensor:
image

There are several intervals at zero-flow, in that intervals probably the temperature is not well-defined and the result depends on the numerical implementation/approximation/tolerance of the solver.

Probably this scenario is often present in the Buildings library examples, because a lot of hydraulic circuits may work in zero-flow condition.

@casella
Copy link
Contributor

casella commented Jun 13, 2024

@mwetter this is a completely general problem: if you add these signals to the reference signals for regression testing, you're always going to get in trouble when doing cross-tool validation

If there are no flow reversals through the sensor (as it seems from the simulation results), the other solution is to disallow flow reversal on the sensor, so it always gets the upstream enthalpy via inStream. Maybe that's the best solution in this case. What do you think?

If you agree with that (I think it's already been done in a previous ticket), I guess @AndreaBartolini can collect all these cases and you can fix them in one shot throughout the whole library.

@AndreaBartolini
Copy link
Contributor Author

Actually the flow reversal is not allowed in the sensor, maybe something should be fixed in the sensor itself:
image

@AndreaBartolini
Copy link
Contributor Author

I'll check it ASAP

@mwetter
Copy link
Member

mwetter commented Jun 13, 2024

This sensor is configured to use inStream() as it has allowFlowReversal=false, and it has tau=1. Hence the output signal is computed as shown below, where the absolute value is computed using regStep. Unless tau is too small compared to the numerical noise, this should be well behaved.

image

@casella
Copy link
Contributor

casella commented Jun 13, 2024

@AndreaBartolini please check with the declarative debugger how the sensor output is computed and report.

@AndreaBartolini
Copy link
Contributor Author

Here the analysis:
the derivative of the sensor output is calculated by the following assignment:
regular: (assign) der(senTSup.T) := (senTSup.T_b_inflow - senTSup.T) * senTSup.k

and the initial value is given by the following assignment:
initial: (assign) senTSup.T := senTSup.T_start

the senTSup.T_b_inflow is calculated by the medium:

(assign) senTSup.T_b_inflow := senTSup.Medium.temperature($cse27)
$cse27 := senTSup.Medium.setState_phX(senTSup.port_a.p, senTSup.port_b.h_outflow, {});

where senTSup.port_b.h_outflow is defined in the modelica model as
port_b.h_outflow = inStream(port_a.h_outflow);

that in the running model becomes the following assignment, that involves just the three-way upstream valve val:
(assign) senTSup.port_b.h_outflow := bui.disFloCoo.val.vol.dynBal.U / bui.disFloCoo.val.vol.dynBal.m

some additional notes:

  1. the port_a of the sensor is connected as following:
    image

so I suppose that the inStream(port_a.h_outflow) should depend not only from the three-way upstream valve but also from what is connected to the outside connector port_a, maybe that after the back-end simplifications the only dependency is from that valve,

  1. the variable bui.disFloCoo.val.vol.dynBal.U has only the initial assignment but it is not clear where is calculated (maybe in a linear system not reported by the debugger...):
    image

the initial assignment is the following:
initial (assign) bui.disFloCoo.val.vol.dynBal.U := bui.disFloCoo.val.vol.dynBal.m * bui.disFloCoo.senTSup.port_b.h_outflow

The only equation that uses it is that has been reported above.

@AndreaBartolini
Copy link
Contributor Author

@casella let me know if you need more detailed investigation.

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

4 participants