bramvdh91 / modelica-ibpsa

Modelica libraries that are used and/or developed within IEA EBC Annex 60
0 stars 0 forks source link

Wrong use of tau for reverse flow #36

Closed marcusfuchs closed 7 years ago

marcusfuchs commented 8 years ago

I had a look at the calculations of tau in PDETime_massFlow and PDETime_massFlowMod. For a mix of one-directional flow and zero-mass-flow, I think that PDETime_massFlowMod yields correct results, while PDETime_massFlow seems a bit off in most cases. Yet, for reverse flow, it seems to me that the code section

if v>= 0 then
    tau = tau_b_lim;
  else
    tau = tau_a_lim;
  end if;

has no effect (neither on results nor performance) and tau = tau_b_lim for all flow regimes, irrespective of the flow direction.

@carlesRT : Can you confirm this finding, or am I making a mistake here? In case the calculation of tau really has that bug, I would suggest to output tau_design_flow = tau_b_lim and tau_reverse_flow = tau_a_lim separately in order to save us an if-else-clause.

In addition, I am currently looking for ways to reduce the number of state events generated by the pipe model, as I expect large costs in CPU time for larger networks from them. Is anyone else currently looking into this?

bramvdh91 commented 8 years ago

I'm not currently looking into CPU aspects, but it seems that in comparison to the MSL pipe, our model is faster (not that this should keep us from looking into this). I do have to familiarize myself again with the current implementation and the differences with the modified delay tracker, as this is not fresh in my mind anymore...

marcusfuchs commented 8 years ago

Yes, my impression is also that the model is quite fast in use cases where there are few instances of it. I am just afraid of many state events in larger networks, but I'll have to run a few tests.

bramvdh91 commented 8 years ago

I have also had another look into the glitchy behaviour of the delay time tau, and it seems that the glitches always appear in case of a reverse flow that's being stopped. However, the exact moment when the glitch appears is not fixed (usually a bit after the beginning of the declining pressure difference, but sometimes also before it). Furthermore, there is no clear connection to events being generated.

Finally, I noticed that events are being generated during zero flow periods between two reverse flow periods. I'm not sure where they are coming from...

bramvdh91 commented 8 years ago

I just had an idea: we're trying our best to get one delay that is always correct, but why not look at it differently? If we would have one delay parameter for design flow and one for reverse, we do not have to worry about all the different cases; the design delay only needs to be correct when there is a positive flow, the reverse one only for reverse. In d7fb544 I have tried to implement this. I chucked out all event checks, just left the spatialDistribution and compare its two outputs to the current time without all tracking difficulties. I have checked it in comparison to the unmodified delay operator in Annex60.Experimental.Pipe.Examples.Comparisons.UCPipeB02Mod. For Dassl and Lsodar, issues appear again, but when using fixed step integrators, it works quite well. Furthermore, the amount of events seems much less. There are still some issues when the flow reverses relative to the direction before zero flow (as we saw in the beginning), but I'm wondering if we should really worry about this...

marcusfuchs commented 8 years ago

Finally, I noticed that events are being generated during zero flow periods between two reverse flow periods. I'm not sure where they are coming from...

Without having looked to deeply into this, I would guess that this is because the boolean to decide on flow direction in our spatialDistribution operator in Annex60.Experimental.Pipe.BaseClasses.PDETime_massFlow is set to u >= 0. This should generate an event when reverse flow turns to zero flow and another event when going back to reverse flow. Interestingly, there is a ticket on JModelica's trac at https://trac.jmodelica.org/ticket/5060 that shows how to suppress this event, but I does not seem to work when I try noEvent(u>=0) in Dymola 2017.

In any case, this behavior could also be an argument to go back to two taus so that each can claim the zero flow periods as its own without interfering events.

carlesRT commented 8 years ago

For a mix of one-directional flow and zero-mass-flow, I think that PDETime_massFlowMod yields correct results, while PDETime_massFlow seems a bit off in most cases.

As shown in #37 it is the other way arround. 'PDETime_massFlow' yields correct results.

it seems to me that the code section 'if v>= 0 then tau = tau_b_lim; else tau = tau_a_lim; end if;'

has no effect (neither on results nor performance) and tau = tau_b_lim for all flow regimes, irrespective of the flow direction.

The implementation is not working correctly. During periods where v<0 happends that tau_a_lim=tau_b_lim that's why these lines seem to be superfluos. To define tau_design_flow = tau_b_lim and tau_reverse_flow = tau_a_lim might work but it will be necessary to define which tau has to be used.

the design delay only needs to be correct when there is a positive flow, the reverse one only for reverse.

Indeed is being difficult to make the implementation work... @bramvdh91 Do you mean, to focus in one-directional flow, preventing the user to use the model with reverse flow? (In this case we might have to add an assert/warning to avoid the model to have reverse flow).

bramvdh91 commented 8 years ago

@carlesRT I mean that by tracking two delays for the same pipe (in pseudocode):

tauRev = time - timeOut_rev    // Reverse flow delay
tau = time - timeOut_des         // Design flow delay

and feeding tauRev to the reverse heat loss calculation and tau to the forward heat loss calculation, there will be no need to make one tau correct for both flow directions. Instead, we track the two and allow them to differ. Since the heat loss blocks will only be active for one flow direction, it will always have the correct delay. What we could try to improve is the behaviour in zero flows as we did before, but it seems that this implementation is much simpler in terms of events.

bramvdh91 commented 7 years ago

This discussion can be closed after merging #72