mom-ocean / MOM6

Modular Ocean Model
Other
185 stars 232 forks source link

attach_cell_methods() treats v_extensive=.false. differently than the optional argument not being present #1632

Open mnlevy1981 opened 4 months ago

mnlevy1981 commented 4 months ago

I'm trying to register a whole slew of diagnostics provided by MARBL, where we want MARBL tell us which ones are vertically extensive. 3D fields that are registered with v_extensive=.false. have cell_methods: xh:mean yh:mean area:mean rather than the expected cell_methods: xh:mean yh:mean zl:mean area:mean. @klindsay28 and I think the problematic block of code is in MOM_diag_mediator:attach_cell_methods:

    if (present(v_cell_method)) then
      if (present(v_extensive)) call MOM_error(FATAL, "attach_cell_methods: " // &
           'Vertical cell method was specified along with the vertically extensive flag.')
      if (len(trim(v_cell_method))>0) then
        if (axes%rank==1) then
          call get_MOM_diag_axis_name(axes%handles(1), axis_name)
        elseif (axes%rank==3) then
          call get_MOM_diag_axis_name(axes%handles(3), axis_name)
        endif
        call MOM_diag_field_add_attribute(id, 'cell_methods', trim(axis_name)//':'//trim(v_cell_method))
        ostring = trim(adjustl(ostring))//' '//trim(axis_name)//':'//trim(v_cell_method)
      endif
    elseif (present(v_extensive)) then
      if (v_extensive) then
        if (axes%rank==1) then
          call get_MOM_diag_axis_name(axes%handles(1), axis_name)
        elseif (axes%rank==3) then
          call get_MOM_diag_axis_name(axes%handles(3), axis_name)
        endif
        call MOM_diag_field_add_attribute(id, 'cell_methods', trim(axis_name)//':sum')
        ostring = trim(adjustl(ostring))//' '//trim(axis_name)//':sum'
      endif
    else
      if (len(trim(axes%v_cell_method))>0) then
        if (axes%rank==1) then
          call get_MOM_diag_axis_name(axes%handles(1), axis_name)
        elseif (axes%rank==3) then
          call get_MOM_diag_axis_name(axes%handles(3), axis_name)
        endif
        call MOM_diag_field_add_attribute(id, 'cell_methods', trim(axis_name)//':'//trim(axes%v_cell_method))
        ostring = trim(adjustl(ostring))//' '//trim(axis_name)//':'//trim(axes%v_cell_method)
      endif
    endif

Note that if v_extensive = .false. then it is present so we go down the elseif (present(v_extensive)) then but there is no else clause to if (v_extensive) then so nothing is done. On the other hand, if v_extensive is not present then we execute the final else clause:

      if (len(trim(axes%v_cell_method))>0) then
        if (axes%rank==1) then
          call get_MOM_diag_axis_name(axes%handles(1), axis_name)
        elseif (axes%rank==3) then
          call get_MOM_diag_axis_name(axes%handles(3), axis_name)
        endif
        call MOM_diag_field_add_attribute(id, 'cell_methods', trim(axis_name)//':'//trim(axes%v_cell_method))
        ostring = trim(adjustl(ostring))//' '//trim(axis_name)//':'//trim(axes%v_cell_method)
      endif

I think we want the above code to execute when v_extensive = .false. is explicitly stated.

mnlevy1981 commented 4 months ago

The easiest fix is probably to have a v_extensive_loc variable defined with

v_extensive_loc = .false.
if (present(v_extensive)) v_extensive_loc = v_extensive

and then

      call MOM_diag_field_add_attribute(id, 'cell_methods', trim(axis_name)//':'//trim(v_cell_method))
        ostring = trim(adjustl(ostring))//' '//trim(axis_name)//':'//trim(v_cell_method)
      endif
-   elseif (present(v_extensive)) then
-     if (v_extensive) then
+   elseif (v_extensive_loc) then
      if (axes%rank==1) then
        call get_MOM_diag_axis_name(axes%handles(1), axis_name)
      elseif (axes%rank==3) then
        call get_MOM_diag_axis_name(axes%handles(3), axis_name)
      endif
      call MOM_diag_field_add_attribute(id, 'cell_methods', trim(axis_name)//':sum')
      ostring = trim(adjustl(ostring))//' '//trim(axis_name)//':sum'
-     endif
    else
      if (len(trim(axes%v_cell_method))>0) then
        if (axes%rank==1) then

(note I took liberties with the whitespace between those two changes)