Last edited [[Timestamp]]

[[PageOutline]]

== For completion by the Sci/Tech/Code reviewer ==
'''Reviewer:''' Rachid Benshila (CNRS)

=== Ticket Details, Documentation and Code changes ===
|| Do you understand the area of code being altered and the reasoning why it is being altered? || YES ||
|| Do the proposed code changes correspond with the stated reason for the change? || YES ||
|| Is the in-line documentation accurate and sufficient? || YES ||
|| Do the code changes comply with NEMO coding standards? || YES ||
|| Is the Ticket documented with sufficient detail for others to understand the impact of the change? || YES ||
|| Does any corresponding external documentation require updating? || YES ||
|| If yes, which docs and have the updates been drafted? || NO ||
|| Are namelist changes required for this change? || NO ||
|| If yes, have they been done? || NA ||
|| Has a completed Ticket Summary template been appended to the ticket to aid code reviews || YES ||
|| Does this summary correspond with your understanding of the full ticket? || YES ||

Ticket, Documentation and Code comments

This development deals deals with tools development, not directly with NEMO code. Policy about tools development is a bit fuzzy, but I guess this code fits  NEMO standard with incore documentation, integration inside standard compilation environment. 

Short documentation could be added in NEMO documentation (mpp part or specific annex).

This piece of code (REBUILD_NEMO) should at some point be used in replacement of the current one (REBUILD) which should be removed.

'''Testing'''

|| Has the NVTK and other jobs been tested with this change? || NA ||
|| Have the required bit comparability tests been run? || NA ||
|| Can this change be shown to have a null impact? (if option not selected) || YES ||
|| If no, is reason for the change valid/understood? || NA ||
|| If no, ensure that the ticket details the impact this change will have on model configurations . || NA ||
|| Is this change expected to preserve all diagnostics? || NA ||
|| If no, is reason for the change valid/understood? || NA ||
|| Are there significant changes in run time/memory? || YES ||



Testing Comments

Since it's a tool, NVTK and others are to applicable here.

Add specific testing comments here

Various tests have been performed on intel with ifort compiler and IBM with xlf:

- first without OpenMP : works fine for different kind of files (restart, outputs), if the size of the variable to rebuild doesn't exceed half of the RAM memory. For instance it fails in the case of
a 3D variable of dimension 300*300*50 with daily outputs during on year on a 8Gb computer. The variable size is around 4Gb, and has to be allocated twice, for the global array to be rebuilt and the sum of the local arrays.[[BR]]
Note that the old rebuild is using a time loop to write the fileds record by record and save memory.  [[BR]]
- with OpenMP and ifort : it fails at first and i had to declare indlimlens as SHARED. After that change, it runs, but the rebuilt fields are not correct. I had it working in commenting the nested OpenMP loops and using OpenMP only on the number of input files. [[BR]]
- with OpenMP and xlf: it runs fine with 3D variables, 2D variables are not correct (to be re-tested by me). It also fails with big arrays

'''Code Review'''

|| Do the code changes comply with NEMO coding standards? || YES ||
|| Are code changes consistent with the design of NEMO? || YES ||
|| Is the code free of unwanted TABs? || YES ||
|| Has the code been wholly (100%) produced by NEMO developers working on NEMO? || YES ||
|| If no, ensure collaboration agreement has been added to the ticket keywords || ||

Add specific code comments or suggested alterations here.

=== Review Summary ===
I don't thing OpenMP problems are major. For ifort, it may be due to the support of nested OpenMP directives n the given implementation, the 2D variables problem has tobe re-checked.
I've got more concern with the memory issue, even if it's strongly related to the configuration in use.

=== Approval for the trunk ===
YES/NO

The code reviewer may approve the change for the NEMO trunk when:

 1. their requests/comments have been addressed satisfactorily.
 1. the above check-list has been completed.

or the code reviewer may choose to reject & assign the change back to the code author.