Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#6203 closed defect (fixed)

Some parameters seem to be never assigned in Buildings models

Reported by: Francesco Casella Owned by: Per Östlund
Priority: blocker Milestone: 1.18.0
Component: New Instantiation Version:
Keywords: Cc: Adrian Pop

Description (last modified by Francesco Casella)

Please check Buildings.Fluid.Actuators.Valves.Examples.ThreeWayValves. The simulation fails at runtime with

Jacobian determinant is NaN.
nonlinear system 56 fails: at t=0

I tried to investigate the issue using the debugger, I found the following chain of equations and assignments
Equation 56 has one equation in the unknown valTab.res3.m_flow

(residual) valTab.res3.dp - homotopy(Buildings.Fluid.BaseClasses.FlowModels.basicFlowFunction_m_flow(valTab.res3.m_flow, valTab.res3.k, valTab.res3.m_flow_turbulent), valTab.res3.dp_nominal_pos * valTab.res3.m_flow / valTab.res3.m_flow_nominal_pos) = 0

valTab.res3.dp_nominal_pos := abs(valTab.res3.dp_nominal)

valTab.res3.dp_nominal := valTab.res3.dpValve_nominal

A similar issue pops up with valTab.res3.m_flow_nominal_pos:

(assign) valTab.res3.m_flow_nominal_pos := abs(valTab.res3.m_flow_nominal)

but then valTab.res3.m_flow_nominal is apparently not assigned in any equation.

This of course explains the NaN in the Jacobian, that would be obtained by computing the 0/0 ratio

Unfortunately, the debugger does not report how valTab.res3.dpValve_nominal is computed. I don't know if this is a problem with the debugger, or if it turns out that this parameter is incorrectly skipped and eventually assumed to be zero.

This issue affects many models in the Buildings library, and is possibly the root cause of #6190

Change History (15)

comment:1 by Francesco Casella, 4 years ago

Milestone: 1.17.01.18.0

comment:2 by Karim Adbdelhak, 4 years ago

NOTE FOR ALL FURTHER LINE AND FILE COMMENTS: I pulled the buildings library from [master at 9.11.2020 https://github.com/lbl-srg/modelica-buildings].

It seems like this is a redeclare bug.

In the main model we have valTab defined with m_flow_nominal=2 (Buildings.Fluid.Actuators.Valves.Examples.ThreeWayValves l:55).

which gets inherited to the component res3 via final m_flow_nominal=m_flow_nominal (Buildings.Fluid.Actuators.BaseClasses.PartialThreeWayValve l:15).

That component res3 gets redefined because it is partial and needs assignment for only one of its attributes (Buildings.Fluid.Actuators.Valves.ThreeWayTable l:6).

This seems to interfere with the inheritance and removes the binding of m_flow_nominal. (Can be seen in instantiated flat model)

Adding , final m_flow_nominal=m_flow_nominal to that redefine statement in (Buildings.Fluid.Actuators.Valves.ThreeWayTable l:6) removes that problem and the value is correctly inherited. So i think this is a problem in our redeclare handling.

I had to add , final m_flow_nominal = m_flow_nominal at multiple places because this occurs for multiple files (full list including the one already mentioned:

  • (Buildings.Fluid.Actuators.Valves.Examples.TwoWayValvesTable l:5)
  • (Buildings.Fluid.Actuators.Valves.Examples.TwoWayValvesTable l:6)
  • (Buildings.Fluid.Actuators.Valves.ThreeWayEqualPercentageLinear l:8)
  • (Buildings.Fluid.Actuators.Valves.ThreeWayEqualPercentageLinear l:10)
  • (Buildings.Fluid.Actuators.Valves.ThreeWayLinear l:5)
  • (Buildings.Fluid.Actuators.Valves.ThreeWayLinear l:7)

The other problem regarding valTab.res3.dpValve_nominal comes from the fact that in our case we have to determine this value implicitely which can be seen in file (Buildings.Fluid.Actuators.BaseClasses.ValveParameters l:50-74) where we are in case 1: CvData == Buildings.Fluid.Types.CvTypes.OpPoint. It is not explicitly declared so it has to be computed from the other equations. Since the default start value is zero it fails with a division to zero in

   Kv_SI = m_flow_nominal/sqrt(dpValve_nominal);

Adding a start value of 1.0 to the definition of dpValve_nominal in (Buildings.Fluid.Actuators.BaseClasses.ValveParameters l:29) removes this problem and the model can be simulated. (maybe another inheritance bug, i don't know if that start value is provided somewhere else). I cannot verify the results, someone else has to do that!

So compiling all of this into one sentence: Mostly seems like a redeclare bug and maybe a modeling error. (arguably one could say that we should not have a min value of 0 for this value and that compilers should use that value in case there is no start value)

Version 0, edited 4 years ago by Karim Adbdelhak (next)

comment:3 by Karim Adbdelhak, 4 years ago

Cc: Adrian Pop added

comment:4 by Karim Adbdelhak, 4 years ago

@adrpo I added you since this seems like a redeclare problem, see comment above!

comment:5 by Karim Adbdelhak, 4 years ago

Owner: changed from Karim Adbdelhak to Per Östlund
Status: newassigned

comment:6 by Karim Adbdelhak, 4 years ago

@perost Since this seems to be redeclare i will assign it to you. :)

comment:7 by Francesco Casella, 4 years ago

For the record, I referred to Buildings 7.0.0 (see the report filename). The master version is a moving target, we don't want to chase that for the time being.

Anyway, this should probably make no difference in this case, as long as the model is still there.

comment:8 by Francesco Casella, 4 years ago

Component: BackendNew Instantiation

comment:9 by Francesco Casella, 4 years ago

The error is now a bit different, but I understand the root cause is the same. If you flatten the model, you see

  parameter Real valTab.res3.m_flow_nominal(quantity = "MassFlowRate", unit = "kg/s") "Nominal mass flow rate";

without any binding, even though the declaration of valTab definitely sets a value to it

  Buildings.Fluid.Actuators.Valves.ThreeWayTable valTab(
    redeclare package Medium = Medium,
    m_flow_nominal=2,
  ...

@perost, could you please investigate?

comment:10 by massimo ceraolo, 4 years ago

Summary: Some parameters seems to be never assigned in Buildings modelsSome parameters seem to be never assigned in Buildings models

in reply to:  10 comment:11 by Francesco Casella, 4 years ago

Replying to ceraolo:
Amazing how we Italians always have a problem with the damn 's' :)

comment:12 by Francesco Casella, 4 years ago

This is the top priority issue among the Buildings tickets, because it is probably causing some failures in the backend due to structural singularites caused by parameters which are wrongly evaluated to zero.

I would suggest to fix it with the highest priority.

comment:13 by Per Östlund, 4 years ago

Resolution: fixed
Status: assignedclosed

Fixed in f92fb52, see report.

There were three regressions in Buildings 7.0.0, but those models had issues before and I think they are now flattened more correctly. Besides that there's now 29 more models simulating including the one mentioned in this ticket.

comment:14 by Francesco Casella, 4 years ago

Description: modified (diff)

comment:15 by Francesco Casella, 4 years ago

Excellent! I just updated the link in the ticket to point to the master branch, so it shows the model running successfully.

I saw the regressions but I'm not too worried about them, we'll look into them when more of the tickets have been resolved.

Note: See TracTickets for help on using tickets.