Issue 35 co2 interactive

[Rob Waters (RobWatersMet)]

PR Summary

Sci/Tech Reviewer:
Code Reviewer: Sam Clarke-Green (@t00sa)

so the CO2 environmental field is held in a co2_interactive variable

REAL, ALLOCATABLE, TARGET, SAVE, PUBLIC :: co2_interactive(:,:,:)
  ! mass_fraction_of_carbon_dioxide_in_air [CF] {UM:00252}

CO2 is only added to fld_names if ukca_config%l_chem_environ_co2_fld

! CO2 field is required if option to use an external CO2 field is set
IF (ukca_config%l_chem_environ_co2_fld .AND. ANY(speci(:) == 'CO2       ')) THEN
  n = n + 1
  IF (n <= n_max) fld_names(n) = fldname_co2_interactive
END IF

It is then allocated

CASE (fldname_co2_interactive)
  CALL set_field_3d_real(i_field, i1, i2, j1, j2, k1, k2, field_data,          &
                         co2_interactive)

But then in ukca_main1 it is passed in unconditionally into ukca_chemistry_ctl and ukca_chemistry_ctl_full as an input variable. Inside both ctl files it has the following intent:

REAL, INTENT(IN) :: co2_interactive(tot_n_pnts)

so it is an explicit shaped array dummy variable - which needs to have been allocated.

Fortran standards:

• An unallocated allocatable variable does not represent an array object.
• A non-allocatable dummy argument requires an actual argument that is an object of the appropriate type, rank, etc.
• Therefore, an unallocated allocatable cannot be associated with a non-allocatable dummy.

However some compilers consider it fine and others don't - seems like GNU in particular doesn't allow it.

Code Quality Checklist

(Some checks are automatically carried out via the CI pipeline)

• I have performed a self-review of my own code
• My code follows the project's style guidelines
• Comments have been included that aid undertanding and enhance the
  readability of the code
• My changes generate no new warnings

Testing

• I have tested this change locally, using the UKCA rose-stem suite
• If shared files have been modified, I have run the UM and LFRic Apps rose
  stem suites
• If any tests fail (rose-stem or CI) the reason is understood and
  acceptable (eg. kgo changes)
• I have added tests to cover new functionality as appropriate (eg. system
  tests, unit tests, etc.)

trac.log

Security Considerations

• I have reviewed my changes for potential security issues
• Sensitive data is properly handled (if applicable)
• Authentication and authorisation are properly implemented (if applicable)

Performance Impact

• Performance of the code has been considered and, if applicable, suitable
  performance measurements have been conducted

AI Assistance and Attribution

• Some of the content of this change has been produced with the assistance
  of Generative AI tool name (e.g., Met Office Github Copilot Enterprise,
  Github Copilot Personal, ChatGPT GPT-4, etc) and I have followed the
  Simulation Systems AI policy
  (including attribution labels)

Documentation

• Where appropriate I have updated documentation related to this change and
  confirmed that it builds correctly

Sci/Tech Review

• I understand this area of code and the changes being added
• The proposed changes correspond to the pull request description
• Documentation is sufficient (do documentation papers need updating)
• Sufficient testing has been completed

Please alert the code reviewer via a tag when you have approved the SR

Code Review

• All dependencies have been resolved
• Related Issues have been properly linked and addressed
• CLA compliance has been confirmed
• Code quality standards have been met
• Tests are adequate and have passed
• Documentation is complete and accurate
• Security considerations have been addressed
• Performance impact is acceptable

[Rob Waters (RobWatersMet)]

Option 1 - Always allocate it

• one solution would be to always have it allocated
• original branch had the following solution

! The co2_interactive array must be allocated before being passed to
! ukca_chemistry_ctl
IF (ALLOCATED(co2_interactive)) THEN
  temporary_co2_array=.FALSE.
ELSE
  ALLOCATE(co2_interactive(1,1,1))
  temporary_co2_array=.TRUE.
END IF

• I don't think we need to allocate it as a 1,1,1
• I think we can also do better

Option 2 - Make it a module variable

• current solution in column mode - call the co2_interactive variable from the module level

USE ukca_environment_fields_mod, ONLY: co2_interactive

• not preferred - unclear dependency

Option 3 - Make the dummy allocatable (preferred option)

real, allocatable, intent(in) :: co2_interactive(row_length,rows,model_levels)

• cleanest solution in my opinion
• note we have to have consistent ranks between dummy variable and actual variable
• hence it can no longer be co2_interactive(:) in the intent
• slightly more code required later.

[Rob Waters (RobWatersMet)]

reshaping required & allocation guard added

  ! CO2 as species
  IF (ANY(speci(:) == 'CO2       ')) THEN
    !  Copy the CO2 concentration into the asad module as VMR
    IF (ukca_config%l_chem_environ_co2_fld) THEN

      ! co2_interactive should be allocated if config option on
      IF (.NOT. ALLOCATED(co2_interactive)) THEN
        errcode = 1
        CALL ereport(ModuleName//':'//RoutineName, errcode,                    &
          'ERROR: co2_interactive array not allocated')
      END IF

      co2_1d(:) = RESHAPE(co2_interactive(:,:,k), [theta_field_size]) / c_co2
    ELSE
      co2_1d(:) = rmdi
    END IF

  END IF

I've also made it consistent against all 3 interfaces.