Opened 9 years ago

Closed 8 years ago

Last modified 3 years ago

#936 closed Bug (fixed)

bugs & reproducibility in closea

Reported by: beppe Owned by: nemo
Priority: low Milestone:
Component: OCE Version: release-3.4
Severity: Keywords: closed_seas reproductibility seas
Cc:

Description

There are 2 bugs in the routine sbc_clo (closea.F90) when the closed seas are active (nn_closea=1 in the namelist):
Bug 1: there is a DO loop inside an IF statement (branch ncstt(jc)==2) but it should be the other way around (i.e. the IF should be inside the DO loop).
Bug 2: Excess water is redistributed on the global ocean including the closed seas. It should be redistributed on the global ocean EXCEPT on the closed seas.
This implies that the water budget of the closed seas is still different from zero after the redistribution, triggering the redistribution at every model time step rather than only at the SBC update time step.
Moreover this implies that the redistribution is not conservative (water is artificially created or destroyed), although the imbalance is very small, about 2 order of magnitude smaller than the global mean surface water budet (but it may be of concern for very long runs without any fresh water budget control, i.e. in coupled runs).
This is fixed accumulating the amount of water redistributed for every closed sea (new variable zcorr in the fixed version in attachment) and removing it from each closed sea after the redistribution.

Finally the sbc_clo subroutine does not give reproducible results when the closed seas are active (nn_closea=1) and the model is compiled with the CPP key key_mpp_rep. This is due to:

  1. the global sum used to compute the global ocean area which doesn't use the glob_sum function (from lib_fortran)
  2. the computation of the closed sea area and water budget which is not mpp_rep compliant (since the closed seas can be split across different MPI processes and the sum here is just on the closed seas so we cannot use glob_sum)

The first issue is fixed using the function glob_sum from lib_fortran.
The second issue is fixed implementing a mpp_rep compliant version of the sbc_clo subroutine which is selected by the CPP key key_mpp_rep. This requires the DDPDD subroutine to be declared PUBLIC in lib_fortran when the CPP key key_mpp_rep is defined.

The fixed closea.F90 and lib_fortran.F90 are in attachment.

Commit History (2)

ChangesetAuthorTimeChangeLog
3433cetlod2012-07-19T09:29:05+02:00

trunk:update offline according to changes done in relation with ticket #936

3421charris2012-07-02T16:44:12+02:00

#936 Changes as suggested by Gurvan et al (testing details in the ticket). Note that for ORCA1 results will change with either nn_closea=0 or 1 because the Caspian Sea is now coded as a closed sea.

Attachments (10)

closea.F90 (25.0 KB) - added by beppe 9 years ago.
Fixed closea.F90
lib_fortran.F90 (18.6 KB) - added by beppe 9 years ago.
closea.2.F90 (20.2 KB) - added by gm 9 years ago.
correction in closea.F90 (merge the 2 clo_sea routines)
lib_fortran.2.F90 (18.6 KB) - added by gm 9 years ago.
made available DDDD routine in all cases
domzgr.F90 (80.6 KB) - added by gm 9 years ago.
use of clo_bat in domzgr
dom_oce.F90 (21.5 KB) - added by gm 9 years ago.
replace nclosea by nn_closea everywhere
domain.F90 (15.7 KB) - added by gm 9 years ago.
replace nclosea by nn_closea everywhere
sbcmod.F90 (22.9 KB) - added by gm 9 years ago.
replace nclosea by nn_closea everywhere
sbcrnf.F90 (26.9 KB) - added by gm 9 years ago.
replace nclosea by nn_closea everywhere
closea.3.F90 (20.2 KB) - added by beppe 8 years ago.

Download all attachments as: .zip

Change History (25)

Changed 9 years ago by beppe

Fixed closea.F90

Changed 9 years ago by beppe

comment:1 Changed 9 years ago by gm

I and Andrew have a careful look on the issues you rise. We totally agree on both bugs identification and solution.

We still have a few issues.

*The main one is that your solution of the BUG 2, leads to a duplication of clo_sea routine (with and without mop_rep… This is problematic for code maintenance. The solution proposed is a merge of the 2 clo_sea routines using lk_mpp_rep where it is need (see closea.F90 lib_fortran in attachment). Note that tis requires to make DDPDD routine available in any cases.
*The second is a small think: rsmall = 1.E-20_wp (or rsmall= 1.D-20_dp but double precision is not required in closea.F90) otherwise compile error:

error #6016: If both kind and exponent letter are present, exponent letter must be an E (R416.1)   [1.D-20]
      REAL(wp), PARAMETER ::   rsmall = 1.D-20_wp    ! Closed sea correction epsilon

*The others points are not related to what you have done.

-There is another bug (missing point in fact) in that routine: in case of ORCA1 configuration , the closed sea of the configuration is not defined. We add it ((see closea.F90 in attachment)
-In closea, there is a routine clo_bat which is not used, but should be used in domzgr.F90 Add the call there and remove the corresponding do loop.
-the last point is stylistic. It concerns nclosea the old name list name of nn_closea. It still appears in a few routines: dom_oce.F90 (where it is defined), domain.F90 (where it is set to nn_closea value) and sbcmod.F90 & sbcrnf.F90 (where it is used instead of nn_closea). We change all these modules (see attachment).

If you agree with our changes, you can commit the correction into the trunk.

Gurvan & Andrew

Changed 9 years ago by gm

correction in closea.F90 (merge the 2 clo_sea routines)

Changed 9 years ago by gm

made available DDDD routine in all cases

Changed 9 years ago by gm

use of clo_bat in domzgr

Changed 9 years ago by gm

replace nclosea by nn_closea everywhere

Changed 9 years ago by gm

replace nclosea by nn_closea everywhere

Changed 9 years ago by gm

replace nclosea by nn_closea everywhere

Changed 9 years ago by gm

replace nclosea by nn_closea everywhere

comment:2 Changed 9 years ago by charris

Gurvan - are these changes definitely tested and ready to be committed to the trunk?

comment:3 Changed 8 years ago by charris

I've tested these code changes in several different ORCA025 configurations and I do now get the same results for all decompositions. As expected there is no change in results with nn_closea=0 (although ORCA1 users who might have nn_closea=1 in namelists need to be aware of the new code introduced for this configuration).

I haven't done any other checks that the code is doing what is intended - it would be good if the original reporter could verify this by re-running whatever tests showed up the problem initially.

Thanks,

Chris

comment:4 Changed 8 years ago by beppe

Dear Gurvan, your changes make totally sense to me.
I'll be able to test the newer version in the ORCA2 configuration next week.

I'll report back to you.

comment:5 Changed 8 years ago by beppe

I've tested the new code changes in the ORCA2 configuration (no sea ice) using the current trunk.

Both problems (mpp reproducibility and water redistribution over closed seas) are fixed now.

comment:6 Changed 8 years ago by charris

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

Changes now on the trunk in [3421].

comment:7 Changed 8 years ago by beppe

  • Resolution fixed deleted
  • Status changed from closed to reopened

Minor bug in closea when key_mpp_rep is not active. Avoid mpp_sum of the global area (surf(jpncs+1)) cause it is already computed using glob_area . Attached a fixed version.

Changed 8 years ago by beppe

comment:8 Changed 8 years ago by beppe

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

Changes now on the trunk in [3441] and [3442].

comment:9 Changed 4 years ago by nicolasmartin

  • Keywords closea removed

comment:10 Changed 4 years ago by nicolasmartin

  • Keywords nemo_v3_4* added

comment:11 Changed 4 years ago by nicolasmartin

  • Keywords seas added; sea removed

comment:12 Changed 4 years ago by nicolasmartin

  • Keywords closed_seas added; closed removed

comment:13 Changed 3 years ago by nemo

  • Keywords release-3.4* added; nemo_v3_4* removed

comment:14 Changed 3 years ago by nemo

  • Keywords reproductibility added; reproducibility removed

comment:15 Changed 3 years ago by nemo

  • Keywords release-3.4* removed
Note: See TracTickets for help on using tickets.