MPAS-Dev / MPAS-Model

Repository for MPAS models and shared framework releases.
238 stars 317 forks source link

Add mpas_modify_att() subroutines to mpas_attlist #1094

Closed dimomatt closed 1 year ago

dimomatt commented 1 year ago

CF compliance requires certain attributes to be updated periodically throughout a run, or at the beginning of a run after compilation. The current attlist module only supports creation and retrieval of attributes. This commit adds the functionality to modify attributes and some basic tests for those routines. The subroutines work by traversing the linked list until they find a node who's attName matches the attName to be changes. It then checks the attType of the node to make sure that the correct type of value is being passed in to the attribute. The error code is then set to 0 and the subroutine exits.

It is also noted that these procedures are not currently threadsafe. Future work may include threadsafe attribute modification and a more comprehensive test suite for all of the attlist procedures.

mgduda commented 1 year ago

@dimomatt No need to adopt any of the following, but it's just an idea that came to mind.

Currently, the logic requires setting ierr in multiple places and it has multiple return statements:

    if (present(ierr)) ierr = 0

    ! Return an error if there is somehow no attList
    if (.not. associated(attList)) then
      ierr = 1
      return
    else
      ! Traverese list looking for attName
      cursor => attlist
      do while (associated(cursor))
        if (trim(cursor % attName) == trim(attName)) then
          if (cursor % attType /= MPAS_ATT_TEXT) then
            if (present(ierr)) ierr = 1
          else
            write(cursor % attValueText,'(a)') trim(attValue)
          end if
          return
        end if
        cursor => cursor % next
      end do
      if (present(ierr)) ierr = 1
    end if

As an alternative, the following structure seems simpler to my mind:

    if (present(ierr)) ierr = 1

    ! Traverese list looking for attName
    cursor => attlist
    do while (associated(cursor))
      if (trim(cursor % attName) == trim(attName)) then
        if (cursor % attType == MPAS_ATT_TEXT) then
          if (present(ierr)) ierr = 0
          write(cursor % attValueText,'(a)') trim(attValue)
        end if
        return
      end if
      cursor => cursor % next
    end do

I haven't actually tested the above... but the idea is to set ierr initially to a bad return code, and to only change it in case we succeed in modifying the requested attribute. There is only one return statement, and we make use of the fact that setting a pointer (cursor) to an unassociated pointer (potentially attlist) results in that pointer also being unassociated.