uDALES / u-dales

uDALES: large-eddy-simulation software for urban flow, dispersion and microclimate modelling
https://udales.github.io/u-dales
GNU General Public License v3.0
47 stars 17 forks source link

In moddriver, <var>mdriver is not set to <var>0driver at simulation start #88

Closed samoliverowens closed 3 years ago

samoliverowens commented 3 years ago

When running driven simulations, I noticed that the temperature would go to zero on the first timestep due to the fact that thlmdriver is initially set to zero. This occurs because of the condition rk3step == 1 in the following sequence in drivergen, which is called by boundary before time-stepping begins (when the condition is false).

      if (rk3step == 1) then
        umdriver = u0driver
        vmdriver = v0driver
        wmdriver = w0driver
        !e12mdriver = e120driver
        if (ltempeq) then
          thlmdriver = thl0driver
        end if
        if (lmoist) then
          qtmdriver = qt0driver
        end if
        if (nsv>0) then
          svmdriver = sv0driver
        end if
      end if

I have simply changed the condition to be rk3step .le. 1, but maybe this isn't optimal.

samoliverowens commented 3 years ago

I have looked at this further after observing example sim 502 seemingly running ok - the simulation probably won't crash unless there are blocks right next to the inlet at i=2. If there are, then in ibnorm the following line

thlm(il, jl:ju, kl:ku) = thlm(il - 1, jl:ju, kl:ku)

sets thlm to zero inside the block (because initially thlm(ib,:,:) = thlmdriver = 0).

But even if the blocks are not next to the inlet, I think this will still cause errors.

bss116 commented 3 years ago

@tomgrylls noted that the buildings for driven simulations as 502 cannot be too close to the downwind edge of the domain, see: https://github.com/uDALES/u-dales/blob/master/docs/udales-simulation-setup.md#running-driven-simulations. Would it be sufficient to just extend the docs there and say that buildings also must not be too close to the inlet? Generally, we need to think about where to put this information, currently the udales-simulation-setup.md document is not referenced anywhere in the other docs..

samoliverowens commented 3 years ago

Maybe, but this is numerical, whereas the reason that blocks can't be too close to the downwind edge is more physical right? As it's to do with the wakes of the last row of blocks.

bss116 commented 3 years ago

yes good point, if there's an issue in the implementation then that needs to be addressed.

tomgrylls commented 3 years ago

@samoliverowens not sure if I fully understand your comment here. So you think the problem is just the very first time that this is called? i.e. time = 0?

My understanding of this is that we are in the drivergen subroutine, which is a subroutine called every timestep when doing driver simulations and this loop is more specifically under an if statement that is true when we are loading the driver data. The loop in question occurs at the end of this if statement. What it does is set the m component to be equal to the 0 component of the arrays then used as upwind boundary conditions when rk3step == 1 - beginning of each time step to be consistent with the RK scheme. The 0 components are set to non-zero values via the above set of loops that perform interpolation to the nearest driver time step.

Even in the first time step, and when this called by boundary before timestepping begins, I think that thlmdriver will be set to equal to thl0driver, which will have been defined by interpolation/ extrapolation in the loops above. I then think this value will be overwritten when timestepping begins. So I do not see where we get the zero value you mentioned?

Please correct me if I have misunderstood. But I think that that the correction of rk3step .le. 1 is not right as on all of the other times that this is called we want this to happen? Have you done any testing by outputting values to check this?

samoliverowens commented 3 years ago

@tomgrylls essentially I think we want to set thlmdriver = thl0driver initially (before timestepping) - would you agree?

tomgrylls commented 3 years ago

@samoliverowens Agreed - okay I see, so it is because rk3step is 0 at startup. Then yes your solution works or setting rk3step = 1 as default if I have understood the issue now. Your solution is probably better in that it will definitely not affect other parts of the code.

samoliverowens commented 3 years ago

@samoliverowens Agreed - okay I see, so it is because rk3step is 0 at startup. Then yes your solution works or setting rk3step = 1 as default if I have understood the issue now. Your solution is probably better in that it will definitely not affect other parts of the code.

Ok I'll put in a PR with this change - I agree that it making rk3step = 1 at startup may cause problems elsewhere.