New URL for NEMO forge!   http://forge.nemo-ocean.eu

Since March 2022 along with NEMO 4.2 release, the code development moved to a self-hosted GitLab.
This present forge is now archived and remained online for history.
ticket/0704/Review – NEMO
wiki:ticket/0704/Review

Version 2 (modified by gm, 14 years ago) (diff)

--

Last edited Timestamp?

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 reviewsNO
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.
  2. the above check-list has been completed.

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