Opened 15 months ago

Last modified 8 months ago

#2462 assigned Task

restart read write with XIOS

Reported by: andmirek Owned by: andmirek
Priority: low Milestone:
Component: MULTIPLE Version: trunk
Severity: minor Keywords:
Cc: Branch review: failed
MP ready?: no Task progress: ready

Commit History (36)

ChangesetAuthorTimeChangeLog
13970andmirek2020-12-02T10:56:33+01:00

Ticket #2462 into the trunk

13964andmirek2020-12-02T09:20:46+01:00

Ticket #2462: Update documentation

13941andmirek2020-12-01T16:48:17+01:00

Ticket #2462: Update test branch

13940andmirek2020-12-01T16:43:03+01:00

Ticket #2462: Changes because of the change in domain definition for XIOS

13934andmirek2020-12-01T13:17:32+01:00

Ticket #2462: update test branch

13933andmirek2020-12-01T09:37:52+01:00

Ticket #2462: Merge with trunk 1378

13932andmirek2020-12-01T09:21:09+01:00

Ticket #2462: Addressing reviewr comments stage 2

13871andmirek2020-11-25T12:51:29+01:00

Ticket #2462: Addressing reviewr comments stage 1

13765andmirek2020-11-10T09:53:53+01:00

Ticket #2462: new sette for test branch

13764andmirek2020-11-10T09:45:05+01:00

Ticket #2462: New test branch

13763andmirek2020-11-10T09:44:10+01:00

Ticket #2462 delete previous test branch

13756andmirek2020-11-09T17:16:03+01:00

Ticket #2462: set correct sette version in test branch

13755andmirek2020-11-09T17:14:02+01:00

Ticket #2462: Sette to test restarts read/write with XIOS

13754andmirek2020-11-09T17:11:35+01:00

Ticket #2462 merge with default sette

13751andmirek2020-11-09T16:48:35+01:00

Ticket #2462: Update test branch to the revision 13750 of the development branch

13750andmirek2020-11-09T16:35:44+01:00

Ticket #2462: Fixes after merge

13728andmirek2020-11-05T17:06:42+01:00

Ticket #2462: merge changes 13702,13717,13718 from #2386

13727andmirek2020-11-05T15:18:53+01:00

Ticket #2462: Upate to trunk rev 13688

13034andmirek2020-06-04T10:07:51+02:00

Ticket #2462 update test branch with changes from dev branch

13033andmirek2020-06-03T17:48:20+02:00

Ticket #2462 remove debug prints

13023andmirek2020-06-03T16:24:02+02:00

Ticket #2462 update test branch with change 13022 in development branch

13022andmirek2020-06-03T16:20:56+02:00

Ticket #2462 update routine setting fields in the file read by XIOS.

13001andmirek2020-06-01T16:04:39+02:00

ticket #2462 update test branch with changes from dev branch

13000andmirek2020-06-01T16:00:13+02:00

Ticket #2462 restart read/write with XIOS in ORCA2_OFF_PISCES

12999andmirek2020-06-01T12:52:45+02:00

Ticket #2462 update sette to handle restart read/write with XIOS

12998andmirek2020-06-01T12:47:36+02:00

Ticket #2462 new sette in test branch

12997andmirek2020-06-01T10:53:29+02:00

Ticket #2462: sette to test restart read/write with XIOS

12987andmirek2020-05-28T10:24:30+02:00

Ticket #2462 test branch with changes in sette to read/write restart with XIOS

12977andmirek2020-05-27T09:35:03+02:00

Ticket #2462 read restart with XIOS in SAS

12969andmirek2020-05-26T12:05:09+02:00

ticket #2462: read restart with XIOS independently for each component

12961andmirek2020-05-22T13:51:12+02:00

Ticket #2462: read/write restart with XIOS in TOP (with debug print statements)

12957andmirek2020-05-20T18:53:27+02:00

Ticket #2462: write/read SI3 restart with XIOS (has debug print statements)

12951andmirek2020-05-19T13:02:48+02:00

Ticket #2462: remove commentted out testing lines

12950andmirek2020-05-19T12:53:16+02:00

Ticket #2462: new XIOS restart read/write interfaces

12914andmirek2020-05-12T15:59:32+02:00

Ticket #2462: merge changes from ticket #2386 to fix XIOS write-read restart

12913andmirek2020-05-12T15:41:54+02:00

Ticket #2462: development branch

Change History (43)

comment:1 Changed 15 months ago by andmirek

In 12913:

Ticket #2462: development branch

comment:3 Changed 15 months ago by andmirek

In 12914:

Ticket #2462: merge changes from ticket #2386 to fix XIOS write-read restart

comment:4 Changed 14 months ago by andmirek

In 12950:

Ticket #2462: new XIOS restart read/write interfaces

comment:5 Changed 14 months ago by andmirek

In 12951:

Ticket #2462: remove commentted out testing lines

comment:6 Changed 14 months ago by andmirek

In 12957:

Ticket #2462: write/read SI3 restart with XIOS (has debug print statements)

comment:7 Changed 14 months ago by andmirek

restart in trdmxl_trc_rst.F90 doesn't follow convention in other parts of NEMO. As a result the approach used to write restart with XIOS can't be used. Reading with XIOS still can be implemented.

comment:8 Changed 14 months ago by andmirek

In 12961:

Ticket #2462: read/write restart with XIOS in TOP (with debug print statements)

comment:9 Changed 14 months ago by andmirek

In 12969:

ticket #2462: read restart with XIOS independently for each component

comment:10 Changed 14 months ago by andmirek

In 12977:

Ticket #2462 read restart with XIOS in SAS

comment:11 Changed 14 months ago by andmirek

In 12997:

Ticket #2462: sette to test restart read/write with XIOS

comment:12 Changed 14 months ago by andmirek

In 12998:

Ticket #2462 new sette in test branch

comment:13 Changed 14 months ago by andmirek

In 12999:

Ticket #2462 update sette to handle restart read/write with XIOS

comment:14 Changed 14 months ago by andmirek

In 13000:

Ticket #2462 restart read/write with XIOS in ORCA2_OFF_PISCES

comment:15 Changed 14 months ago by andmirek

In 13001:

ticket #2462 update test branch with changes from dev branch

comment:16 Changed 14 months ago by andmirek

the code was changed this test is not valid

Last edited 14 months ago by andmirek (previous) (diff)

comment:17 Changed 14 months ago by andmirek

In 13022:

Ticket #2462 update routine setting fields in the file read by XIOS.

comment:18 Changed 14 months ago by andmirek

In 13023:

Ticket #2462 update test branch with change 13022 in development branch

comment:19 Changed 14 months ago by andmirek

In 13033:

Ticket #2462 remove debug prints

comment:20 Changed 14 months ago by andmirek

In 13034:

Ticket #2462 update test branch with changes from dev branch

comment:21 Changed 14 months ago by andmirek

  • Status changed from new to assigned
  • Task progress changed from Unspecified to ready

Current code is : NEMO/branches/2020/test_12905_xios_restart @ r13034 ( last change @ r13034 )

SETTE validation report generated for :

NEMO/branches/2020/test_12905_xios_restart @ r13034 (last changed revision)

on XC40_METO_IFORT arch file

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

!——restart——!

WGYRE_PISCES_ST run.stat restartability passed : 13034
WGYRE_PISCES_ST tracer.stat restartability passed : 13034
WORCA2_ICE_PISCES_ST run.stat restartability passed : 13034
WORCA2_ICE_PISCES_ST tracer.stat restartability passed : 13034
WORCA2_OFF_PISCES_ST tracer.stat restartability passed : 13034
WAMM12_ST run.stat restartability passed : 13034
WORCA2_SAS_ICE_ST run.stat restartability passed : 13034
WAGRIF_DEMO_ST run.stat restartability passed : 13034
WSPITZ12_ST run.stat restartability passed : 13034
WISOMIP_ST run.stat restartability passed : 13034
WOVERFLOW_ST run.stat restartability passed : 13034
WLOCK_EXCHANGE_ST run.stat restartability passed : 13034
WVORTEX_ST run.stat restartability passed : 13034
WICE_AGRIF_ST run.stat restartability passed : 13034

!——repro——!

WGYRE_PISCES_ST run.stat reproducibility passed : 13034
WGYRE_PISCES_ST tracer.stat reproducibility passed : 13034
WORCA2_ICE_PISCES_ST run.stat reproducibility passed : 13034
WORCA2_ICE_PISCES_ST tracer.stat reproducibility passed : 13034
WORCA2_OFF_PISCES_ST tracer.stat reproducibility passed : 13034
WAMM12_ST run.stat reproducibility passed : 13034
WORCA2_SAS_ICE_ST run.stat reproducibility passed : 13034
WORCA2_ICE_OBS_ST run.stat reproducibility passed : 13034
WAGRIF_DEMO_ST run.stat reproducibility passed : 13034
WSPITZ12_ST run.stat reproducibility passed : 13034
WISOMIP_ST run.stat reproducibility passed : 13034
WVORTEX_ST run.stat reproducibility passed : 13034
WICE_AGRIF_ST run.stat reproducibility passed : 13034

!——agrif check——!

ORCA2 AGRIF vs ORCA2 NOAGRIF run.stat unchanged - passed : 13034 13034

!——result comparison check——!

check result differences between :
VALID directory : /home/d02/mandrej/nemo_validation_RRW/NEMO_VALIDATION at rev 13034
and
REFERENCE directory : /home/d02/mandrej/nemo_validation/NEMO_VALIDATION at rev 12913

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

comment:22 Changed 9 months ago by andmirek

In 13727:

Ticket #2462: Upate to trunk rev 13688

comment:23 Changed 9 months ago by andmirek

In 13728:

Ticket #2462: merge changes 13702,13717,13718 from #2386

comment:24 Changed 9 months ago by andmirek

In 13750:

Ticket #2462: Fixes after merge

comment:25 Changed 9 months ago by andmirek

  • Description modified (diff)

comment:26 Changed 9 months ago by andmirek

  • Description modified (diff)

comment:27 Changed 9 months ago by andmirek

In 13751:

Ticket #2462: Update test branch to the revision 13750 of the development branch

comment:28 Changed 9 months ago by andmirek

In 13754:

Ticket #2462 merge with default sette

comment:29 Changed 9 months ago by andmirek

In 13755:

Ticket #2462: Sette to test restarts read/write with XIOS

comment:30 Changed 9 months ago by andmirek

In 13756:

Ticket #2462: set correct sette version in test branch

comment:31 Changed 9 months ago by andmirek

In 13763:

Ticket #2462 delete previous test branch

comment:32 Changed 9 months ago by andmirek

In 13764:

Ticket #2462: New test branch

comment:33 Changed 9 months ago by andmirek

In 13765:

Ticket #2462: new sette for test branch

comment:34 Changed 9 months ago by smasson

Branch Review

I am happy to see that you get read of the long hard coded lists of restart variables and that you now follow what is done for restarts written without xios.
I think the code structure is now close to be ok.
However, to me, the version I reviewed (branches/2020/dev_12905_xios_restart@13750) is still containing many “small” things (some of them dating from before the branch creation) that should be improved/cleaned.
To me, this branch does not correspond yet to the standards required for a finalized branch ready to be merged.
As you we see bellow in the details comments, I propose modifications that would simplify and clarify your code, keep the number of modified routines as small as possible (basically only the iom routines).
I definitely think that the less you modify things, the better it is.

My main issue is the multiplication of variables and routines calls that seams to be useless or could be avoided (as far I as understand it).
The chosen variables names are often not easy to understand.
To me this makes the things more complicated than they should be.

The starting point is these 2 namelist parameters:

   ln_xios_read = .false.  !  use XIOS to read restart file (only for a single file restart)
   nn_wxios = 0      !  use XIOS to write restart file 0 - no, 1 - single file output, 2 - multiple file output
  • I agree with the choice of adding 2 parameters.
  • I know the history and understand the motivations but It think It would be better if they had similar names. One day we could also use xios to read restarts in multiple files. The name should also include the idea of restart.

⇒ Why not using something like: nn_rst_rxios and nn_rst_wxios ?

I am concern by the variables you next add in iom_def, in_out_manager and a few other places. I don’t understand their need. This makes the code harder to follow.

  • nxioso: i don’t understand the need of this variable. For me it is just a duplication of nn_wxios
  • lwxios is just the same as nn_wxios > 0
  • lrxios is just the same as ln_xios_read.AND.ln_rstart. Why not forcing ln_xios_read = .false. when ln_rstart = .false. : ln_xios_read = ln_xios_read .and. ln_rstart = .true.
  • lxios_sini: i don’t understand this variable. We read only “one_file” restarts so why having this additional switch?
  • lroxios, lrixios, lrtxios, lrsxios: i don’t understand the need of these variables. All component are writing their restart the same way according to ln_xios_read. Why do you need specific variables for each component?
  • you added the ldxios optional argument in the calls to iom_get and to iom_rstput (changing a few hundred lines of code!). I don’t understand why you need to do this and change all theses lines.

⇒ for iom_get: to define llxios, you can: (1) you test ln_xios_read, (2) you compare kiomid to numror, numrir, etc…
⇒ for iom_rstput: to define llx, you can: (1) you test nn_wxios, (2) you compare kiomid to numrow, numriw, etc…

Every time you read/write restarts, you swap xios context to select the proper restart context. I am wondering if this could not be done in iom_get and iom_rstput interfaces. As I just said above, using the namelist parameters and comparing kiomid to numror, numrir, etc… you now what you want to do so you could determine at this point if you have to switch context or not.
This would allow to remove the calls to iom_swap that you had in many places.
⇒ Suppressing the ldxios parameter and the calls to iom_swap would, I guess, suppress (almost?) all the modifications you done outside of iom routines. This would greatly reduce the impact of this development on the code, simplifying its implementation, its understanding and its maintenance.

Regarding the Context names, I agree, these are needed.
Originally we had only cxios_context. Now that we have 9 contexts, we need to differentiate their names more easily.
I like your idea to start with cr or cw. Next I propose to add who is doing what: all, oce, ice, top, sed. Next the type of file: out or rst and ending by cxt instead of xios_context which is too long
This would give the following variables names:

cw_allout_cxt
cw_ocerst_cxt, cr_ocerst_cxt
cw_icerst_cxt, cr_icerst_cxt
cw_toprst_cxt, cr_toprst_cxt
cw_sedrst_cxt, cr_sedrst_cxt

Other points (not to be neglected)

  • very strange indentation in many iom routines. non-respect of “NEMO coding style"
  • using nf90_* functions in iom is not clean. This should be done only in iom_nf90. cn_var and dimsz of the file_descriptor should be used.
  • following this idea, idfp should be suppressed and you should directly use numror
  • same idea: why do you use fname in iom_set_vars_active? You should directly use iom_file(numror)%name.
  • in iom_set_vars_active, meta is a local variable that is not used anywhere else. Why is it declared in iom_def.F90 and not simply locally?
  • There id many calls to the TRIM function that are useless. There is no need to use TRIM when comparing 2 character varables. I also thing there is no need to use TRIM when passing a character as an argument to XIOS routines.
  • It several places, there are variables declared but not used. I noticed for example
    CHARACTER(lc)  ::   clpath
     INTEGER :: i
     CHARACTER(len=1024) :: fname
     CHARACTER(len=lc)   :: axis_ref
    

comment:35 Changed 8 months ago by andmirek

In 13871:

Ticket #2462: Addressing reviewr comments stage 1

comment:36 Changed 8 months ago by andmirek

In 13932:

Ticket #2462: Addressing reviewr comments stage 2

comment:37 Changed 8 months ago by andmirek

In 13933:

Ticket #2462: Merge with trunk 1378

comment:38 Changed 8 months ago by andmirek

As of 01/12/2020 I can't run sette test. The error

Config file (bld): /data/d02/mandrej/NEMO_2020_DEVELOPMENT/dev_12905_xios_restart/mk/cpp.fcm
ERROR: /data/d02/mandrej/NEMO_2020_DEVELOPMENT/dev_12905_xios_restart/mk/bldxag.cfg: LINE 17:
       /data/d02/mandrej/NEMO_2020_DEVELOPMENT/dev_12905_xios_restart/ext/PPR/src: source does not exist or is not readable.
ERROR: /data/d02/mandrej/NEMO_2020_DEVELOPMENT/dev_12905_xios_restart/mk/bldxag.cfg: LINE 39:
       bld::pp::ppr_1d: invalid sub-package in declaration.
ERROR: /data/d02/mandrej/NEMO_2020_DEVELOPMENT/dev_12905_xios_restart/mk/bldxag.cfg: LINE 42:
       bld::tool::fppflags::ppr_1d: invalid sub-package in declaration.

comment:39 Changed 8 months ago by andmirek

In 13934:

Ticket #2462: update test branch

comment:40 Changed 8 months ago by andmirek

In 13940:

Ticket #2462: Changes because of the change in domain definition for XIOS

comment:41 Changed 8 months ago by andmirek

In 13941:

Ticket #2462: Update test branch

comment:42 Changed 8 months ago by andmirek

In 13964:

Ticket #2462: Update documentation

comment:43 Changed 8 months ago by andmirek

In 13970:

Ticket #2462 into the trunk

Note: See TracTickets for help on using tickets.