id	summary	reporter	owner	description	type	status	priority	milestone	component	version	resolution	keywords	cc
550	IMPORTANT: Updated TS_MPDATA, biology, sediment, difussion	arango	arango	"Several updates are carried out:

 * The '''TS_MPDATA''' option is updated so it is also included in the tracer predictor step in '''pre_step3d.F'''. Previously, '''TS_MPDATA''' was only applied in the corrector step in '''step3d_t.F'''.  This cleaned out the awkward logic in all the biological models, sediment model, and horizontal tracer difussion routines.  There is not longer a need to update '''n+1/2''' time level in the tracer array, which is stored in '''t(:,:,:,3,itrc)'''.  For example, the following statement is eliminated from the biological models:
{{{
#ifdef TS_MPDATA
              t(i,j,k,3,ibio)=t(i,j,k,nnew,ibio)*Hz_inv(i,k)
#endif
}}}
 Because of this change, we not longer can reproduce identical solutions when '''TS_MPDATA''' is activated.

 * Added first-order horizontal and vertical advection to '''pre_step3d.F''' (predictor step) when '''TS_MPDATA''' is activated. Many thanks to John Warner for working and testing the logic for this capability.

 * Corrected the Fennel biological model, '''fennel.h''', to compute the correct source/sink term due to biogeochemical processes. Notice that all the biological model in ROMS use the old time level ('''nstp''') concentration to initialize the source/sink terms.  Therefore, the change due to biogeochemical processes is at the end of the biological model is:
{{{
        DO itrc=1,NBT
          ibio=idbio(itrc)
          DO k=1,N(ng)
            DO i=Istr,Iend
              cff=Bio(i,k,ibio)-Bio_old(i,k,ibio)
              t(i,j,k,nnew,ibio)=t(i,j,k,nnew,ibio)+cff*Hz(i,j,k)
            END DO
          END DO
        END DO
}}}
  '''ALL the biological models in ROMS must have this update at the end'''!!! This maybe not obvious to everybody, but it is the correct way to update the tracer array with the source/sink contribution for time-step '''nnew'''.  This is due to ROMS design and separate time-stepping scheme for each tracer term. Notice that we subtract the '''Bio_old''' quantity, which have the value at time-step '''nstp'''. This also takes into account any constraints (non-negative concentrations, concentration limiters) specified before entering biogeochemical kernel. If '''Bio''' were unchanged by biogeochemical processes, the increment would be exactly zero. Notice that final tracer values, '''t(:,:,:,nnew,:)''' are not bounded >=0 so that we can '''preserve''' total inventory of nutrients when advection causes tracer concentration to go negative.  Many thanks to John Wilkin for his persistence trying to figure out why there were a mismatch in the tracer diagnostic budgets.  This is the only way that a problem like this can be detected.  By the way, the other biological models in this repository are correct. We just missed this problem in the '''fennel.h'''.  It has been like this for several years. 

 * Corrected a shared-memory problem in the random walk of Lagrangian floats, '''vwalk_floats.F'''.  Many thanks to Mark Hadfield for [https://www.myroms.org/forum/viewtopic.php?f=19&t=2584 bringing] this problem to my attention.

 * Allowed '''nAVG''' and '''nDIA''' to be '''one'''. Although this logic is weird, I can see circumstances were such capability maybe useful. Many thanks to Changwei Bian [https://www.myroms.org/forum/viewtopic.php?f=19&t=2589 reporting] this problem.

 * Corrected a bug in '''def_rst.F''' for '''PERFECT_RESTART''' and '''WET_DRY'''.  The NetCDF variable '''wetdry_mask_rho''' has the wrong dimensions. We need to use '''sr2dgrd''' instead:
{{{
         status=def_var(ng, iNLM, RST(ng)%ncid, RST(ng)%Vid(idRwet),     &
ncname,      &
      &                 NF_FOUT, nvd3, sr2dgrd, Aval, Vinfo, ncname,     &
      &                 SetFillVal = .FALSE.)
}}}
  Many thanks to Turuncoglu and Kate Hedstrom [https://www.myroms.org/forum/viewtopic.php?f=24&t=1853 reporting and tracking] this problem.

'''WARNING:''' '''TS_MPDATA''' can only used in distributed_memory ('''MPI''') for more that one thread. There is a parallel bug in serial with partitions and shared-memory.  I have been tracking this on and off for more than a year.  I think that it is a problem is the implementation of '''TS_MPDATA'''.  It needs to be split in couple of parallel regions. I will address this issue in the future when I will revise the '''OpenMP''' directives."	upgrade	closed	major	Release ROMS/TOMS 3.6	Nonlinear	3.6	Fixed