-
Notifications
You must be signed in to change notification settings - Fork 5
Enable no local timesteping #356
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
base: main
Are you sure you want to change the base?
Enable no local timesteping #356
Conversation
…oes local timesteps but in all cells.
FlorisBuwaldaDeltares
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the whole local timestepping business but apart from 1 suggestion and 1 question the code looks good. If @V-Chavarrias agrees with the implementation then go ahead and merge
| integer, intent(in) :: nsubsteps !< number of substeps | ||
| integer, intent(in) :: limtyp !< limited higher-order upwind (>0) or first-order upwind (0) | ||
| integer, dimension(Ndx), intent(in) :: jaupdate !< cell updated (1) or not (0) | ||
| integer, dimension(Lnx), intent(out) :: jaupdatehorflux !< update horizontal flux (1) or not (0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| integer, dimension(Lnx), intent(out) :: jaupdatehorflux !< update horizontal flux (1) or not (0) | |
| integer, dimension(Lnx), intent(out) :: jaupdatehorflux !< horizontal flux on link needs updating (1) or not (0) |
|
|
||
| if (timon) call timstrt("get_jaupdatehorflux", ithndl) | ||
|
|
||
| if (nsubsteps == 1) then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is not clear to me why this check (and the corresponding input argument) could be removed. It is also not clear to me why they were replaced by a check on limtyp.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you only have one substep, you need to update the fluxes.
Originally, this was done in subroutine get_jaupdatehorflux (this if-clause). However, I guess that somebody found it was better to bypass the calling altogether and made the same if-clause in update_constituents:
if (nsubsteps > 1) then
call get_jaupdatehorflux(nsubsteps, limtyp, jaupdate, jaupdatehorflux)
end if
Hence the one inside get_jaupdatehorflux is redundant.
V-Chavarrias
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
| end if | ||
|
|
||
| call prop_set(prop_ptr, 'numerics', 'TransportAutoTimestepdiff', jatransportautotimestepdiff, 'Auto Timestepdiff in Transport, 0 : lim diff, no lim Dt_tr, 1 : no lim diff, lim Dt_tr, 2: no lim diff, no lim Dt_tr, 3=implicit (only 2D)') | ||
| call prop_set(prop_ptr, 'numerics', 'transportLocalTimeStep', ja_transport_local_time_step, '0=no, 1=yes (default), 2=yes but all cells use the same local time step. Flag for local time stepping in the transport module') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remember to send Priska the new input flags
| call prop_set(prop_ptr, 'Time', 'dtInit', dt_init, 'Initial computation timestep (s)') | ||
|
|
||
| call prop_set(prop_ptr, 'Time', 'timeStepAnalysis', ja_time_step_analysis, '0=no, 1=see file *.steps') | ||
| call prop_set(prop_ptr, 'Time', 'timeStepAnalysis', ja_time_step_analysis, '0=no, 1=yes (default)') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default is 0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created UNST-9399 to explain in manual.
|
|
||
| if (timon) call timstrt("get_jaupdatehorflux", ithndl) | ||
|
|
||
| if (nsubsteps == 1) then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you only have one substep, you need to update the fluxes.
Originally, this was done in subroutine get_jaupdatehorflux (this if-clause). However, I guess that somebody found it was better to bypass the calling altogether and made the same if-clause in update_constituents:
if (nsubsteps > 1) then
call get_jaupdatehorflux(nsubsteps, limtyp, jaupdate, jaupdatehorflux)
end if
Hence the one inside get_jaupdatehorflux is redundant.
| ! write(6,*) dtmin | ||
| ! end if | ||
|
|
||
| if (ja_transport_local_time_step==2) then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be moved right after nsubsteps is computed (new line 71) and prevent the loop on ndxi? E.g.:
else
logtwo = log(2.0_dp)
nsubsteps = max(1, 2**int(log(dts / dtmin) / logtwo + 0.9999_dp))
if (ja_transport_local_time_step==2) then
ndeltasteps = nsubsteps
numnonglobal = ndxi
else
dtmin = dts / nsubsteps
! get number of substeps
do kk = 1, Ndxi
dt = dtmax(kk)
if (dt < dts) then
ndeltasteps(kk) = min(2**int(log(dt / dtmin) / logtwo), nsubsteps)
numnonglobal = numnonglobal + 1
else
ndeltasteps(kk) = nsubsteps
end if
end do
! fictitious boundary cells
do LL = Lnxi + 1, Lnx
ndeltasteps(ln(1, LL)) = ndeltasteps(ln(2, LL))
end do
endif
| nsubsteps = imiss | ||
| ndeltasteps = imiss | ||
| jaupdatehorflux = imiss | ||
| numnonglobal = imiss |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me to remember; nothing to be done with this comment.
imiss is -999. This is filled in alloc_transport:
call realloc(ndeltasteps, Ndx, keepExisting=.false., fill=1)
call realloc(jaupdate, Ndx, keepExisting=.false., fill=1)
call realloc(jaupdatehorflux, Lnx, keepExisting=.false., fill=1)
and numnonglobal in get_ndeltasteps, and it is always set.
| jaupdate = 1 | ||
|
|
||
| jaupdate = 1 ! always update the first step | ||
| jaupdatehorflux=1 ! always update the first step |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not really needed. jaupdatehorflux is used inside comp_fluxhor3D but there there is a check on the number of substimesteps:
if (nsubsteps > 1) then
if (jaupdatehorflux(LL) == 0) then
cycle
else
…ping-in-transport
…nsport' of https://github.com/Deltares/Delft3D into fm/bugfix/UNST-9391-enable-no-local-timestepping-in-transport
…ping-in-transport
…ping-in-transport
…ping-in-transport
What was done
dtmin_transpset now computed in setdtorg()dtsby thedtmin_transpif smaller than the current flow time step.Evidence of the work done
<add video/figures if applicable>
Tests
e02_f022_c107
Documentation
Created issue: UNST-9393
Issue link