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/0681/Review (diff) – NEMO

Changes between Version 1 and Version 2 of ticket/0681/Review


Ignore:
Timestamp:
2010-10-04T19:27:23+02:00 (14 years ago)
Author:
rblod
Comment:

--

Legend:

Unmodified
Added
Removed
Modified
  • ticket/0681/Review

    v1 v2  
    1212- integer in namelist should be nn_something, ex niaufn=nn_iaufn[[BR]] - real in namelist should be rn_something, ex saltfixmin = rn_saltfixmin[[BR]] - character name (here mainly filename) should be cn_something, enactfiles=cn_enactfiles[[BR]] 
    1313 
    14 Presentation of the namelist changed since nemo_v3.1(?), it should be something like (see https://forge.ipsl.jussieu.fr/nemo/browser/trunk/CONFIG/ORCA2_LIM/EXP00/namelist)[[BR]] 
     14Presentation of the namelist changed since nemo_v3.1(?), it should be something like (see [https://forge.ipsl.jussieu.fr/nemo/browser/trunk/CONFIG/ORCA2_LIM/EXP00/namelist)[[BR https://forge.ipsl.jussieu.fr/nemo/browser/trunk/CONFIG/ORCA2_LIM/EXP00/namelist )] 
    1515 
    16 !-------------------------------[[BR]] &namobs        !   observation usage switch[[BR]] !----------------------------------[[BR]] 
    17  
    18   ln_t3d = .true.   !  Logical switch for T profile observations [[BR]] ln_s3d = .true.  ! Logical switch for S profile observations[[BR]] 
     16!-------------------------------[[BR]] &namobs        !   observation usage switch[[BR]] !----------------------------------[[BR]] ln_t3d = .true.   !  Logical switch for T profile observations [[BR]] ln_s3d = .true.  ! Logical switch for S profile observations[[BR]] 
    1917 
    2018Usually, we report the namelist modifications for each standard configuration under CONFIG directory, even when they are not in use and all parameters et to false (it's quite painful to do) 
     
    3028  & psinu, pcosu, psinv, pcosv! copy of data 
    3129 
    32 could be 
    33  
    34 REAL(wp), DIMENSION(jpi,jpj), INTENT( OUT )::  psinu, pcosu, psinv, pcosv! copy of data 
     30could be REAL(wp), DIMENSION(jpi,jpj), INTENT( OUT )::  psinu, pcosu, psinv, pcosv! copy of data 
    3531 
    3632 * OPA_SRC/OBS : [[BR]] header is missing in new routines (see existing routine), sometimes there are reference to NEMOVAR licence[[BR]] 
    3733 
    38   !!----------------------------------------------------------------------[[BR]] 
    39   !! NEMO/OPA 3.3 , LOCEAN-IPSL (2010)[[BR]] 
    40   !! $Id$[[BR]] !! Software governed by the CeCILL licence (modipsl/doc/NEMO_CeCILL.txt)[[BR]]  
    41   !!----------------------------------------------------------------------[[BR]] 
     34  !!----------------------------------------------------------------------[[BR]] !! NEMO/OPA 3.3 , LOCEAN-IPSL (2010)[[BR]] !! $Id$[[BR]] !! Software governed by the CeCILL licence (modipsl/doc/NEMO_CeCILL.txt)[[BR]]  !!----------------------------------------------------------------------[[BR]] 
    4235 
    4336 * same remark as above for some declarations on several lines with &[[BR]] 
     
    5346USE IOIPSL[[BR]]  CALL  ymds2ju (year,month,day,sec,julian)[[BR]] CALL  ju2ymds (julian,year,month,day,sec)[[BR]] 
    5447 
    55  * It may be logical to define kind of data fbsp and fbdp in par_kind.F90 rather than in obs_fbm[[BR]]  
     48 * It may be logical to define kind of data fbsp and fbdp in par_kind.F90 rather than in obs_fbm[[BR]] 
    5649 * Using nf90_open rather than iom_open may be a sources a problem when using nesting (maybe one day) with key agrif 
    5750 
     
    6053It would be nice to have access to an observation file or at least a link to one in the routine 
    6154 
    62  
    6355'''Ticket Details, Documentation and Code changes''' 
    6456 
    65 ||Do you understand the area of code being altered and the reasoning why it is being altered?||YES/NO|| 
    66 ||Do the proposed code changes correspond with the stated reason for the change?||YES/NO|| 
    67 ||Is the in-line documentation accurate and sufficient?||YES/NO|| 
    68 ||Do the code changes comply with NEMO coding standards?||YES/NO|| 
    69 ||Is the Ticket documented with sufficient detail for others to understand the impact of the change?||YES/NO|| 
    70 ||Does any corresponding external documentation require updating?||YES/NO|| 
    71 ||If yes, which docs and have the updates been drafted?||YES/NO|| 
    72 ||Are namelist changes required for this change?||YES/NO|| 
    73 ||If yes, have they been done?||YES/NO|| 
    74 ||Has a completed Ticket Summary template been appended to the ticket to aid code reviews||YES/NO|| 
    75 ||Does this summary correspond with your understanding of the full ticket?||YES/NO|| 
     57||Do you understand the area of code being altered and the reasoning why it is being altered?||YES|| 
     58||Do the proposed code changes correspond with the stated reason for the change?||YES|| 
     59||Is the in-line documentation accurate and sufficient?||YES|| 
     60||Do the code changes comply with NEMO coding standards?||YES|| 
     61||Is the Ticket documented with sufficient detail for others to understand the impact of the change?||YES|| 
     62||Does any corresponding external documentation require updating?||YES|| 
     63||If yes, which docs and have the updates been drafted?||NO|| 
     64||Are namelist changes required for this change?||YES|| 
     65||If yes, have they been done?||NO|| 
     66||Has a completed Ticket Summary template been appended to the ticket to aid code reviews||YES|| 
     67||Does this summary correspond with your understanding of the full ticket?||YES|| 
    7668 
    7769Ticket, Documentation and Code comments 
     
    8173=== Testing === 
    8274||Has the NVTK and other jobs been tested with this change?||YES/NO|| 
    83 ||Have the required bit comparability tests been run?||YES/NO|| 
    84 ||Can this change be shown to have a null impact? (if option not selected)||YES/NO|| 
     75||Have the required bit comparability tests been run?||YES|| 
     76||Can this change be shown to have a null impact? (if option not selected)||YES|| 
    8577||If no, is reason for the change valid/understood?||YES/NO/NA|| 
    8678||If no, ensure that the ticket details the impact this change will have on model configurations .||YES/NO/NA|| 
    87 ||Is this change expected to preserve all diagnostics?||YES/NO|| 
     79||Is this change expected to preserve all diagnostics?||YES|| 
    8880||If no, is reason for the change valid/understood?||YES/NO/NA|| 
    8981||Are there significant changes in run time/memory?||YES/NO||