Last edited [[Timestamp]] [[PageOutline]] == For completion by the Sci/Tech/Code reviewer == '''Reviewer:''' [ Gurvan Madec ] '''Ticket''' #704 '''NEMO Ticket page''' : DOES NOT EXIST ! === 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?||NO|| ||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?||YES/NO|| ||Has a completed Ticket Summary template been appended to the Ticket #704 to aid code reviews||NO|| ||Does this summary correspond with your understanding of the full Ticket #704?||YES/NO|| Ticket, Documentation and Code comments Add specific Ticket, Documentation and code comments here The coding style have to be slightly improved. Suggestions of style modification have been given to the autor Neither the ticket nor the associated (non exixting!) wiki page describe what has been changed and how to activate the changes The paper documentation is missing, but should not be required as there is currently no ice documentation. Nevertheless, a published paper (Bouillon et al ocean modelling 2009) describe in detail what has been done and which effect this has on the model solution. === 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)||NO|| ||If no, is reason for the change valid/understood?||YES|| ||If no, ensure that the ticket details the impact this change will have on model configurations .||YES/NO/NA|| ||Is this change expected to preserve all diagnostics?||YES|| ||If no, is reason for the change valid/understood?||YES/NO/NA|| ||Are there significant changes in run time/memory?||??|| Testing Comments No significant changes in the memory requirement. No information given about the CPU time.... === Code Review === ||Do the code changes comply with NEMO coding standards?||YES|| ||Are code changes consistent with the design of NEMO?||almost 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|||| The development can be better in term of modularity of the code. But this can be done latter on. The improvement will consist in defining the rheology arrays in limrhg module in a new lim_rhg_init routine (not in ice.F90 module) for both VP and EVP case. This reduce the number of time the key_VP appears in the system. Nevertheless, I guess that within 1/2 years, the VP rheology will disappear... So there is no really need to perform this additional work === Review Summary === It is a well done work, the documentation is missing (as for all the LIM 2 and 3 components) but a paper (bouillon et al Ocean Modelling 2009) describe precisely the VP and EVP rheology in LIM and the sensitivity of this change in LIM2 ! This should be sufficient. === Approval for the trunk === YES if the suggested style modifications are done and a NEMOTicket page created on the wiki 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.