Opened 3 years ago
Closed 3 years 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)
Changeset | Author | Time | ChangeLog |
---|---|---|---|
12921 | smasson | 2020-05-14T09:51:23+02:00 | trunk: bugfix potential out-of-bounds in bdydta/bdytides, see #2412 |
Change History (6)
comment:1 Changed 3 years ago by jchanut
comment:2 Changed 3 years ago by smasson
- Cc jchanut girrmann added
comment:3 Changed 3 years 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 years 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 years ago by smasson
In 12921:
comment:6 Changed 3 years 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
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.