Opened 5 months ago

Closed 3 months ago

#2412 closed Bug (fixed)

potential out-of-bounds in bdydta/bdytides?

Reported by: smasson Owned by: systeam
Priority: low Milestone:
Component: BDY Version: trunk
Severity: minor Keywords:
Cc: jchanut, girrmann

Description

Context

Following #2399, I had a closer look at the use of ssh and u2d/u3d arrays in bdy routines and I found that:

in bdydta, we have:

IF( cn_dyn2d(jbdy) == 'frs' ) THEN   ;   ilen1(:)=idx_bdy(jbdy)%nblen(:)
ELSE                                 ;   ilen1(:)=idx_bdy(jbdy)%nblenrim(:)
ENDIF
IF ( dta_bdy(jbdy)%lneed_ssh   ) dta_bdy_s(jbdy)%ssh(1:ilen1(1)) = dta_bdy(jbdy)%ssh(1:ilen1(1))
IF ( dta_bdy(jbdy)%lneed_dyn2d ) dta_bdy_s(jbdy)%u2d(1:ilen1(2)) = dta_bdy(jbdy)%u2d(1:ilen1(2))
IF ( dta_bdy(jbdy)%lneed_dyn2d ) dta_bdy_s(jbdy)%v2d(1:ilen1(3)) = dta_bdy(jbdy)%v2d(1:ilen1(3))

which is coherent with what we have in bdytides

! JC: If FRS scheme is used, we assume that tidal is needed over the whole
! relaxation area      
IF( cn_dyn2d(ib_bdy) == 'frs' ) THEN   ;   ilen0(:) = idx_bdy(ib_bdy)%nblen   (:)
ELSE                                   ;   ilen0(:) = idx_bdy(ib_bdy)%nblenrim(:)
ENDIF

However since I rewrote bdydta,

  • ssh is always allocated and used only on the rim
  • u2d and v2d are used and allocated only on the rim except if ln_full_vel = T, see bdy_dta_init

There is nothing here related to the choice of FRS scheme…

I don't really know bdytides but I suspect we could have a problem here…

Same story in the 4.0-HEAD…

Analysis


Fix


Commit History (1)

ChangesetAuthorTimeChangeLog
12921smasson2020-05-14T09:51:23+02:00

trunk: bugfix potential out-of-bounds in bdydta/bdytides, see #2412

Change History (6)

comment:1 Changed 5 months ago by jchanut

I think that the problem is again that with FRS on 2d mode: you need barotropic velocity data over the whole boundary region. If I understood well the new code, we need an extended provision of data for 2d velocities ONLY if total vs baroclinic velocities are read.
For ssh it is true that it is needed only at rim-points and only with Flather.

comment:2 Changed 3 months ago by smasson

  • Cc jchanut girrmann added

comment:3 Changed 3 months ago by smasson

Jerome, Gaston there is several points I would like to clarify…

1) dta_bdy( : )%ssh is used only over the rim in bdy_dyn2d_fla and in bdy_ssh. It is never used out of the rim. There is no need to allocate, and define it out of the rim. Am I right?

2) dta_bdy( : )%u2d seems to be used over the full bdy in several cases:

  • ln_full_vel = T : which is already taken into account
  • in bdy_dyn2d_frs : with lines 136/144 a do loop on nblen.
  • since [11191] the test on llrim0 makes sure that we call bdy_dyn2d_frs only once.
  • in bdy_dta line 212 : for runoff treatment, we modify u2d over the full bdy. But it will be used out of the rim only if we use frs.
  • I don't really understand bdy_dta_tides… basically it is updating dta_bdy( : )%ssh, dta_bdy( : )%u2d and dta_bdy( : )%v2d, isn't it?

So as far as I understand it:

  • dta_bdy( : )%ssh should be always used only over the rim.
  • For dta_bdy( : )%u2d and v2d, in addition to ln_full_vel = T, we should also consider the case 'frs' for which this field must be defined over the whole bdy and not only over the rim.

comment:4 Changed 3 months ago by jchanut

No need to read ssh over the full relaxation zone indeed. Only over the rim (and it's never a good idea anyway to nudge ssh in the domain interior).
And yes, for frs on the 2d mode, u2d/v2d data is needed over the whole bdy. These are eventually updated by tidal data in bdy_dta_tide. In the later case, again, no need for ssh tidal data out of the rim.

comment:5 Changed 3 months ago by smasson

In 12921:

trunk: bugfix potential out-of-bounds in bdydta/bdytides, see #2412

comment:6 Changed 3 months ago by smasson

  • Resolution set to fixed
  • Status changed from new to closed

This ticket was reviewed by Jérome.
[12921] fixes it. It passes all sette tests, results are unchanged

-bash-4.2$ ./sette_rpt.sh

Current code is : NEMO/trunk @ r12921  ( last change @ r12921 )

SETTE validation report generated for :

       NEMO/trunk @ r12921 (last changed revision)

       on X64_IRENE arch file


!!---------------1st pass------------------!!

   !----restart----!
WGYRE_PISCES_ST              run.stat    restartability  passed :  12921
WGYRE_PISCES_ST              tracer.stat restartability  passed :  12921
WORCA2_ICE_PISCES_ST         run.stat    restartability  passed :  12921
WORCA2_ICE_PISCES_ST         tracer.stat restartability  passed :  12921
WORCA2_OFF_PISCES_ST         tracer.stat restartability  passed :  12921
WAMM12_ST                    run.stat    restartability  passed :  12921
WORCA2_SAS_ICE_ST            run.stat    restartability  passed :  12921
WAGRIF_DEMO_ST               run.stat    restartability  passed :  12921
WSPITZ12_ST                  run.stat    restartability  passed :  12921
WISOMIP_ST                   run.stat    restartability  passed :  12921
WOVERFLOW_ST                 run.stat    restartability  passed :  12921
WLOCK_EXCHANGE_ST            run.stat    restartability  passed :  12921
WVORTEX_ST                   run.stat    restartability  passed :  12921
WICE_AGRIF_ST                run.stat    restartability  passed :  12921

   !----repro----!
WGYRE_PISCES_ST              run.stat    reproducibility passed :  12921
WGYRE_PISCES_ST              tracer.stat reproducibility passed :  12921
WORCA2_ICE_PISCES_ST         run.stat    reproducibility passed :  12921
WORCA2_ICE_PISCES_ST         tracer.stat reproducibility passed :  12921
WORCA2_OFF_PISCES_ST         tracer.stat reproducibility passed :  12921
WAMM12_ST                    run.stat    reproducibility passed :  12921
WORCA2_SAS_ICE_ST            run.stat    reproducibility passed :  12921
WORCA2_ICE_OBS_ST            run.stat    reproducibility passed :  12921
WAGRIF_DEMO_ST               run.stat    reproducibility passed :  12921
WSPITZ12_ST                  run.stat    reproducibility passed :  12921
WISOMIP_ST                   run.stat    reproducibility passed :  12921
WVORTEX_ST                   run.stat    reproducibility passed :  12921
WICE_AGRIF_ST                run.stat    reproducibility passed :  12921

   !----agrif check----!
ORCA2 AGRIF vs ORCA2 NOAGRIF run.stat    unchanged  -    passed :  12921 12921

   !----result comparison check----!

check result differences between :
VALID directory : /ccc/work/cont005/ra0542/massons/NEMO_ALL_VALIDATIONS/trunk/NEMO_VALIDATION at rev 12921
and
REFERENCE directory : /ccc/work/cont005/ra0542/massons/NEMO_ALL_VALIDATIONS/trunk/NEMO_VALIDATION at rev 12895

WGYRE_PISCES_ST       run.stat    files are identical
WGYRE_PISCES_ST       tracer.stat files are identical
WORCA2_ICE_PISCES_ST  run.stat    files are identical
WORCA2_ICE_PISCES_ST  tracer.stat files are identical
WORCA2_OFF_PISCES_ST  tracer.stat files are identical
WAMM12_ST             run.stat    files are identical
WISOMIP_ST            run.stat    files are identical
WORCA2_SAS_ICE_ST     run.stat    files are identical
WAGRIF_DEMO_ST        run.stat    files are identical
WSPITZ12_ST           run.stat    files are identical
WISOMIP_ST            run.stat    files are identical
WVORTEX_ST            run.stat    files are identical
WICE_AGRIF_ST         run.stat    files are identical
Note: See TracTickets for help on using tickets.