ctsl%rof_to_glc needs to be set before fldlist_add calls adding the Fgrg_rof? fields is done

[ekluzek]

94 introduces a control option ctl%rof_from_glc. However, this flag is being set after the fldlist_add calls (which should be inside an if statement for that control option).

    call fldlist_add(fldsToRof_num, fldsToRof, 'Fgrg_rofl') ! liq runoff from glc
    call fldlist_add(fldsToRof_num, fldsToRof, 'Fgrg_rofi') ! ice runoff from glc

So the setting of ctl%rof_from_glc should be set before the above and the above changed to...

    if ( ctl%rof_from_glc ) then
       call fldlist_add(fldsToRof_num, fldsToRof, 'Fgrg_rofl') ! liq runoff from glc
       call fldlist_add(fldsToRof_num, fldsToRof, 'Fgrg_rofi') ! ice runoff from glc
    end if

Doing this will enable MOSART to work with current and previous versions of CISM, right now it will die when it gets to the fldlist_add call as Fgrg_rofl is not being sent from CISM.

[jedwards4b]

@ekluzek Is there a compelling reason to make this compatible with older versions of cism? I don't see a strong argument for backward compatibility.

[ekluzek]

It would've helped with the transition. But moving forward there isn't a compelling need for it.

But, my point is that the logic should be fixed or removed. If we are good with just removing it we could go that route as well and change the name of this.

[mvertens]

@ekluzek - I removed ctl%glc2rof this morning. At this point mosart always sends the glc runoff if the the coupler advertises that it will be sent. However, the field is not actually filled in with non-zero values unless glc -> rof is active and the field actually exists in the glc export:

! custom merge for glc->rof
    ! glc->rof is mapped in med_phases_post_glc
    do ns = 1,is_local%wrap%num_icesheets
      if (is_local%wrap%med_coupling_active(compglc(ns),comprof)) then
        do nf = 1,size(fldnames_fr_glc)
          if ( fldbun_fldchk(is_local%wrap%FBImp(compglc(ns),comprof), fldnames_fr_glc(nf), rc=rc) .and. &
               fldbun_fldchk(is_local%wrap%FBExp(comprof), fldnames_fr_glc(nf), rc=rc) ) then
            call fldbun_getdata1d(is_local%wrap%FBImp(compglc(ns),comprof), &
                 trim(fldnames_fr_glc(nf)), dataptr_in, rc)
            if (chkerr(rc,__LINE__,u_FILE_u)) return
            call fldbun_getdata1d(is_local%wrap%FBExp(comprof), &
                 trim(fldnames_fr_glc(nf)), dataptr_out , rc)
            if (chkerr(rc,__LINE__,u_FILE_u)) return
            ! Determine export data
            if (ns == 1) then
              dataptr_out(:) = dataptr_in(:)
            else
              dataptr_out(:) = dataptr_out(:) + dataptr_in(:)
            end if
          end if
        end do
      end if
    end do

Since this logic has already been removed I'm not sure what the status of this PR should be.

[mvertens]

I meant the issue not the PR.

[ekluzek]

Hmmm. I still see it in what I just updated to. And I see it in the PR as it exists now...

git grep rof_from_glc | more
src/cpl/nuopc/rof_import_export.F90:      ctl%rof_from_glc = .true.
src/cpl/nuopc/rof_import_export.F90:      ctl%rof_from_glc = .false.
src/cpl/nuopc/rof_import_export.F90:      write(iulog,'(A,l1)') trim(subname) //' rof from glc is ',ctl%rof_from_glc
src/cpl/nuopc/rof_import_export.F90:    if (ctl%rof_from_glc) then
src/riverroute/mosart_control_type.F90:     logical :: rof_from_glc                         ! if true, will receive liq and ice runoff from glc
src/riverroute/mosart_driver.F90:      if (ctl%rof_from_glc) then

Is there perhaps some commits that haven't been pushed to your fork? Or outstanding changes that didn't get committed and need to be and pushed?

Assuming that's the case -- we'll just close this one as resolved in #94.

[mvertens]

@ekluzek - Sorry for the confusion. I should not write when its so late. You are right - this is still there. The key point is that I put the ctl%rof_from_glc in order to be backwards compatible with older versions of cmeps. The logic is not in the advertise - but in the realize. If cmeps does not advertise these fields then there is no connection and the fields won't be realized - hence the logic in rof_import_export: realize_fields:

    if (fldchk(importState, 'Fgrg_rofl') .and. fldchk(importState, 'Fgrg_rofl')) then
      ctl%rof_from_glc = .true.
    else
      ctl%rof_from_glc = .false.
    end if

However, i just realized that in older version of cmeps Fgrg_rofl and Fgrg_rofl are not defined in fd_cesm.yaml - so the backwards compatibility will be broken. I think at this point the code is working and I can try to address this issue later. That said - thanks for pointing this out.