Last edited [[Timestamp]]

[[PageOutline]]

== For completion by the Sci/Tech/Code reviewer ==
'''Reviewer:''' Pierre-Antoine Bouttier

=== 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? || Partially, due to the storng.F90 file (see https://forge.ipsl.jussieu.fr/nemo/ticket/1605 ) ||
|| 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? || A chapter in the NEMO book is missing about that module. ||
|| Are namelist changes required for this change? || YES ||
|| If yes, have they been done? || YES (yes, in the CONFIG/SHARED/namelist_ref file) ||
|| Has a completed Ticket Summary template been appended to the ticket to aid code reviews || NO (see https://forge.ipsl.jussieu.fr/nemo/ticket/1605 ) ||
|| Does this summary correspond with your understanding of the full ticket? || NA ||

Ticket, Documentation and Code comments

The main point here is that the documentation in the NEMO book is missing.

=== Testing ===
|| Has the NVTK and other jobs been tested with this change? || YES ||
|| Have the required bit comparability tests been run? || YES ||
|| 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? || YES (if option not selected) ||
|| If no, is reason for the change valid/understood? || NA ||
|| Are there significant changes in run time/memory? || NO (if option not selected) ||

Testing Comments

SETTE tests are all OK when option is disabled. Scientific tests have not been done with this module enabled.

=== Code Review ===
|| Do the code changes comply with NEMO coding standards? || Partially (as previously said) ||
|| 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 ||

Allocation of arrays in the module are not checked during the initialisation phase. This have to be fixed.

Another remark is more about  the software architecture. It seems to be more meaningful to extract STO/ directory from OPA_SRC directory. Indeed, the module is not directly related to the NEMO dynamic core and it is generic enough to be applied to other NEMO components (e.g. LIM2, PISCES)

=== Review Summary ===
Currently, the stochastic parametrization of EOS is implemented. It does not introduce regressive behaviors of the code. 

However, an external documentation is still missing. Also, Scientific tests have not been done. 

Concerning the code itself, arrays allocation have to be checked. The STO directory could be extracted from OPA_SRC.

=== 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.