Last edited [[Timestamp]]

[[PageOutline]]

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

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

Ticket, Documentation and Code comments

'''Reviewer 1''': No specific comments on the documentation.

'''Reviewer 2''': No specific comments on the documentation.

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

Testing Comments

'''Reviewer 1''': The proposed SETTE tests give as outputs the same statistics in ocean.ouput than the previous OBS version on my own machines. 

'''Reviewer 2''': New code can not be tested in our system due to the newer NEMO version. 

Add specific testing comments here

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

'''Reviewer 1''': I have one suggestion, more about the NEMO architecture. It seems to me more meaningful to extract the OBS module from the OPA_SRC directory.[[BR]]First, for me, it is not directly related to the NEMO dynamic core, then it can be used by other NEMO components (Sea-Ice component, for example)

'''Reviewer 2''': One possible code issue in obs_pre_prof has been reported.

=== Review Summary ===
'''Reviewer 1''': These changes correspond clearly to a very welcomed simplification of the OBS module. They are well written, both for code and documentation, according to the NEMO coding conventions.

'''Reviewer 2''': This task is about simplification of the Observation operator code in NEMO. Changes involve merging all observations into two types: surface obs and profile obs. And simplify all corresponding subroutines that read, prepare, calculate model equivalent and write out for these obs types. Test of the new code is not possible in our system but a throughout code check has been carried out and we believe this simplification task has been very well done. 

=== Approval for the trunk ===
'''Reviewer 1''': YES 

'''Reviewer 2''': YES

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.