Constituent updates #549

peverwhee · 2024-03-18T20:37:15Z

Summary

Brings in optional metadata field to enable dynamic (run-time) constituents; also brings in a couple new methods for the constituent object.

closes #557

Description

Key dynamic constituent mods:

ccpp_capgen.py: create dynamic constituent dictionary (key=scheme name) and pass into API()
constituents.py: add handling of dynamic constituents to register constituents routine (including conditional call(s) to user-supplied routines); update error handling for register_constituents

Includes additional constituent object methods:

set_water_species
set_min_val
set_molar_mass

Includes fixes to ensure consistent order of generated code (use statements, variables, etc)

Also brings back the .github/workflow/python.yaml workflow that got lost somehow

User interface changes?: Yes, but optional
User can now use "dynamic_constituent_routine" metadata header field to point to a function contained within the module fortran.

Testing:
Modified advection test to include dynamic constituent routines.
Added doctests to ccpp_datafile

Generated code snippet

   subroutine test_host_ccpp_register_constituents(host_constituents, errflg, errmsg)
      ! Create constituent object for suites in 

      use ccpp_constituent_prop_mod, only: ccpp_constituent_properties_t
      ! Suite constituent interfaces
      use ccpp_cld_suite_cap,        only: cld_suite_constituents_num_consts
      use ccpp_cld_suite_cap,        only: cld_suite_constituents_const_name
      use ccpp_cld_suite_cap,        only: cld_suite_constituents_copy_const
      ! Dynamic constituent routines
      use cld_liq, only: cld_liq_dynamic_constituents
      use cld_ice, only: cld_ice_dynamic_constituents

      ! Dummy arguments
      type(ccpp_constituent_properties_t), target, intent(in)  :: host_constituents(:)
      character(len=512), intent(out)   :: errmsg ! ccpp_error_message
      integer,            intent(out)   :: errflg ! ccpp_error_code
      ! Local variables
      integer                                      :: num_suite_consts
      integer                                      :: num_consts
      integer                                      :: num_dyn_consts
      integer                                      :: index, index_start
      integer                                      :: field_ind
      type(ccpp_constituent_properties_t), pointer :: const_prop
      ! dynamic constituent props variable for cld_ice
      type(ccpp_constituent_properties_t), allocatable :: dyn_const_prop_0(:)
      ! dynamic constituent props variable for cld_liq
      type(ccpp_constituent_properties_t), allocatable :: dyn_const_prop_1(:)

      errflg = 0
      num_consts = size(host_constituents, 1)
      num_dyn_consts = 0
      ! Number of suite constants for cld_suite
      num_suite_consts = cld_suite_constituents_num_consts(errflg=errflg, errmsg=errmsg)
      if (errflg /= 0) then
         return
      end if
      num_consts = num_consts + num_suite_consts
      ! Add in dynamic constituents
      call cld_ice_dynamic_constituents(dyn_const_prop_0, errcode=errflg, errmsg=errmsg)
      if (errflg /= 0) then
         return
      end if
      num_dyn_consts = num_dyn_consts + size(dyn_const_prop_0)
      call cld_liq_dynamic_constituents(dyn_const_prop_1, errcode=errflg, errmsg=errmsg)
      if (errflg /= 0) then
         return
      end if
      num_dyn_consts = num_dyn_consts + size(dyn_const_prop_1)
      num_consts = num_consts + num_dyn_consts
      ! Pack dynamic_constituents array
      allocate(dynamic_constituents(num_dyn_consts), stat=errflg)
      if (errflg /= 0) then
         errmsg = 'failed to allocate dynamic_constituents'
         return
      end if
      index_start = 0
      do index = 1, size(dyn_const_prop_0, 1)
         dynamic_constituents(index + index_start) = dyn_const_prop_0(index)
      end do
      index_start = size(dyn_const_prop_0, 1)
      deallocate(dyn_const_prop_0)
      do index = 1, size(dyn_const_prop_1, 1)
         dynamic_constituents(index + index_start) = dyn_const_prop_1(index)
      end do
      index_start = size(dyn_const_prop_1, 1)
      deallocate(dyn_const_prop_1)
      ! Initialize constituent data and field object
      call test_host_constituents_obj%initialize_table(num_consts)
      ! Add host model constituent metadata
      do index = 1, size(host_constituents, 1)
         const_prop => host_constituents(index)
         call test_host_constituents_obj%new_field(const_prop, errcode=errflg, errmsg=errmsg)
         nullify(const_prop)
         if (errflg /= 0) then
            return
         end if
      end do

      ! Add dynamic constituent properties
      do index = 1, size(dynamic_constituents, 1)
         const_prop => dynamic_constituents(index)
         call test_host_constituents_obj%new_field(const_prop, errcode=errflg, errmsg=errmsg)
         nullify(const_prop)
         if (errflg /= 0) then
            return
         end if
      end do
      ! Add cld_suite constituent metadata
      num_suite_consts = cld_suite_constituents_num_consts(errflg=errflg, errmsg=errmsg)
      if (errflg /= 0) then
         return
      end if
      do index = 1, num_suite_consts
         allocate(const_prop, stat=errflg)
         if (errflg /= 0) then
            errmsg = "ERROR allocating const_prop"
            return
         end if
         call cld_suite_constituents_copy_const(index, const_prop, errflg=errflg, errmsg=errmsg)
         if (errflg /= 0) then
            return
         end if
         call test_host_constituents_obj%new_field(const_prop, errcode=errflg, errmsg=errmsg)
         nullify(const_prop)
         if (errflg /= 0) then
            return
         end if
      end do

      call test_host_constituents_obj%lock_table(errcode=errflg, errmsg=errmsg)
      if (errflg /= 0) then
         return
      end if
! Set the index for each active constituent
      do index = 1, SIZE(test_host_model_const_indices)
         call test_host_constituents_obj%const_index(field_ind,                                   &
              test_host_model_const_stdnames(index), errcode=errflg, errmsg=errmsg)
         if (errflg /= 0) then
            return
         end if
         if (field_ind > 0) then
            test_host_model_const_indices(index) = field_ind
         else
            errflg = 1
            errmsg = 'No field index for '//trim(test_host_model_const_stdnames(index))
            return
         end if
      end do

Allow schema file to be passed to validate Add Fortran comment write method Add metadata table type and name to CCPP datatable variable entries Improve generated constituent-handling code Fix issues with output of long comments Ensure more instance variables are 'private' Have capgen create database object Add line fill (target line length) interface to FortranWriter New Fortran interface, indent_size New Fortran interface, blank_line Added insert function for verbatim copy to file Allow arbitrary break for long lines Make sure call list variables have an intent Added link to constituent dictionary Add metadata to CCPP constituent object DDT Add Fortran writing unit tests and fix line break bugs Added constituent props array host interface, improved unit conversion error Use +ELLIPSIS instead of +IGNORE_EXCEPTION_DETAILS for doctests Constituent cleanup, no function side effects in interface Completed and optimized constituent accessor routines Added minimum allowed value property

Add molecular weight as variable and constituent property Fix missing property (advected) from database

…ariable in hash_table

Rename dir for var_action test to distinguish from ct_build

…d variable arrays

climbfuji

Thanks for addressing my comments. This is a non-expert approval which on its own is not sufficient for merging ;-)

dustinswales

@peverwhee Some minor comments, but overall looks good. Thanks for these changes!
I will start testing in the UFS using this branch and push to get on the commit queue asap.

scripts/ccpp_capgen.py

dustinswales · 2024-05-29T16:49:34Z

scripts/constituents.py

+        cap.write(f"integer{spc} :: num_consts", 2)
+        cap.write(f"integer{spc} :: num_dyn_consts", 2)
+        cap.write(f"integer{spc} :: index, index_start", 2)
+        cap.write(f"integer{spc} :: field_ind", 2)
        cap.write(f"type({CONST_PROP_TYPE}), pointer :: const_prop", 2)


Should this be cap.write(f"type({CONST_PROP_TYPE}), pointer :: const_prop => null()", 2)

src/ccpp_constituent_prop_mod.F90

dustinswales · 2024-05-29T17:08:46Z

src/ccpp_constituent_prop_mod.F90

+         cprop => this%find_const(standard_name)
+         if (associated(cprop)) then
+            ! Standard name already in table, let's see if the existing constituent is the same
+            cindex = cprop%const_index()


test/advection_test/test_host.F90

dustinswales · 2024-05-29T17:23:41Z

test/advection_test/test_host.F90

+         errflg = 0
+      end if
+      ! Check moist mixing ratio for a dynamic constituent
+      call const_props(index_dyn2)%is_dry(const_log, errflg, errmsg)


Should this be calling "is_moist"?

added another testing code block to confirm that is_moist returns true (but keeping the test of is_dry() returning false)

test/unit_tests/sample_scheme_files/duplicate_dyn_const.F90

test/unit_tests/sample_scheme_files/dyn_const_not_present.F90

test/var_compatibility_test/run_test

dustinswales

@peverwhee Thanks for addressing my comments. Looks good!
(Running the UFS tests now, and will open PRs)

grantfirl · 2024-06-06T15:33:31Z

@gold2718 @mwaxmonsky Would you like to re-review before merging this?

mwaxmonsky

Apologies for the delay, looks good to me!

gold2718 · 2024-06-06T17:20:29Z

@gold2718 @mwaxmonsky Would you like to re-review before merging this?

Sorry, can I have today?

grantfirl · 2024-06-06T17:21:51Z

@gold2718 @mwaxmonsky Would you like to re-review before merging this?

Sorry, can I have today?

Sure. We're trying to attach this to a set of UFS PRs that are on a queue to be final tested/merged early next week. If we can get final approval this week, it should be fine.

gold2718

I have not looked at the test code yet but I have a couple of things that look like they should be changed (looking at 6e165e5).

gold2718 · 2024-06-06T20:42:57Z

scripts/constituents.py

+           cap.write("end if", 3)
+           cap.write("end do", 2)
+        # end if
+


Why not deallocate dyn_const_name here? It does not seem to have any other uses and I do not see any call to {host_model.name}_ccpp_deallocate_dynamic_constituents.

we can't deallocate {dyn_const_name} here because that's the module-level variable that holds the data for the dynamic constituents; deallocating it would mean you could no longer access those constituent properties

src/ccpp_constituent_prop_mod.F90

gold2718 · 2024-06-07T14:41:00Z

scripts/constituents.py

+                cap.write(f"do index = 1, size(dyn_const_prop_{idx}, 1)", 2)
+                cap.write(f"{dyn_const_name}(index + index_start) = dyn_const_prop_{idx}(index)", 3)
+                cap.write("end do", 2)
+                cap.write(f"index_start = size(dyn_const_prop_{idx}, 1)", 2)


I see how this works if there are two entries in dyn_const_dict but what about 3? Shouldn't this be:

Suggested change

cap.write(f"index_start = size(dyn_const_prop_{idx}, 1)", 2)

cap.write(f"index_start = index_start + size(dyn_const_prop_{idx}, 1)", 2)

yes good point! fixed

gold2718

I'm still not sure I have covered all the corner cases (a simpler implementation would be easy to verify), but I have not found any more definite problems.

Steve Goldhaber and others added 30 commits July 13, 2022 21:45

Avoid RE deprecation warning by moving case fold to state_machine.py

bce4d22

Fix doctest

279c180

Allow '1' to be a valid unit

057588e

Added molar_mass (molecular weight) value to constituent property type

678621e

initial changes

27f6851

fixes

ca65368

address review comments

10416a9

hopefully final fixes

5e8e33a

Merge pull request NCAR#41 from peverwhee/add_molecular_weight

9bb155b

Add molecular weight as variable and constituent property Fix missing property (advected) from database

initial commit

a2386d6

fix function name

7657f33

fix fstring

5315500

fix mistakes in routine names and comments

84bade6

split up register and initialize

3494311

cleanup

d9871c0

cleanup

a71e1b5

add temporary hash_table variable to avoid dangling pointer

6c7c82a

deallocate hash_table

d4ad4aa

cleaner way to deallocate hash table in iterator object

f549d64

change name of const array interfaces

5813d19

add intialized_in_physics property

8ff8d5a

better error handling; fix for copy constituent

f074f1c

remove unnecessary use statment

5e839bf

fix unit tests

feb8bcb

revert changes for unit conversion test

15f52c6

handle constituent edge cases better and differently; revert module v…

3703645

…ariable in hash_table

fix decimal

1e83c4c

Merged changes from new_variables_input

31919df

Rename dir for var_action test to distinguish from ct_build

remove initialized-in-physics constituent from advection test require…

dad0dbd

…d variable arrays

Courtney Peverley and others added 6 commits May 6, 2024 16:26

Merge remote-tracking branch 'NCAR/main' into group-fix

8c90217

code cleanup; review requests

9e43b78

code clean-up; add test for promoted variable with no horiz dimension

b668b2e

code review; cleanup

1e139b5

Merge remote-tracking branch 'NCAR/main' into add_const_interface

522ad25

only run pytests on test/ directory in python.yaml workflow

15a16ea

peverwhee requested a review from climbfuji May 16, 2024 21:17

climbfuji approved these changes May 17, 2024

View reviewed changes

Courtney Peverley added 2 commits May 20, 2024 15:30

merge in optional fortran v metadata checker

c06600e

Merge remote-tracking branch 'origin/group-fix' into add_const_interface

0b8f23a

This was referenced May 20, 2024

Add script to check fortran vs metadata without a host model #558

Closed

Handle dimensions for promoted variables #562

Closed

mattldawson mentioned this pull request May 28, 2024

Register species from MICM ESCOMP/atmospheric_physics#93

Closed

Merge remote-tracking branch 'NCAR/main' into add_const_interface

c781edc

dustinswales requested changes May 29, 2024

View reviewed changes

address review comments

6e165e5

peverwhee requested a review from dustinswales May 29, 2024 23:10

dustinswales approved these changes May 30, 2024

View reviewed changes

mwaxmonsky approved these changes Jun 6, 2024

View reviewed changes

grantfirl mentioned this pull request Jun 6, 2024

HR4 Gravity Wave Drag Update NOAA-EMC/fv3atm#836

Merged

gold2718 requested changes Jun 6, 2024

View reviewed changes

fix incorrect subname

8297a6b

gold2718 reviewed Jun 7, 2024

View reviewed changes

fix indexing

97c3cc5

peverwhee requested a review from gold2718 June 7, 2024 17:38

gold2718 approved these changes Jun 7, 2024

View reviewed changes

grantfirl merged commit 0f82327 into NCAR:main Jul 10, 2024
19 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Constituent updates #549

Constituent updates #549

peverwhee commented Mar 18, 2024 •

edited

Loading

climbfuji left a comment

dustinswales left a comment

dustinswales May 29, 2024

peverwhee May 29, 2024

dustinswales May 29, 2024

peverwhee May 29, 2024

dustinswales May 29, 2024

peverwhee May 29, 2024

dustinswales left a comment •

edited

Loading

grantfirl commented Jun 6, 2024

mwaxmonsky left a comment

gold2718 commented Jun 6, 2024

grantfirl commented Jun 6, 2024

gold2718 left a comment

gold2718 Jun 6, 2024

peverwhee Jun 6, 2024

gold2718 Jun 7, 2024

peverwhee Jun 7, 2024

gold2718 left a comment

	cap.write(f"index_start = size(dyn_const_prop_{idx}, 1)", 2)
	cap.write(f"index_start = index_start + size(dyn_const_prop_{idx}, 1)", 2)

Constituent updates #549

Constituent updates #549

Conversation

peverwhee commented Mar 18, 2024 • edited Loading

Summary

Description

Generated code snippet

climbfuji left a comment

Choose a reason for hiding this comment

dustinswales left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dustinswales left a comment • edited Loading

Choose a reason for hiding this comment

grantfirl commented Jun 6, 2024

mwaxmonsky left a comment

Choose a reason for hiding this comment

gold2718 commented Jun 6, 2024

grantfirl commented Jun 6, 2024

gold2718 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gold2718 left a comment

Choose a reason for hiding this comment

peverwhee commented Mar 18, 2024 •

edited

Loading

dustinswales left a comment •

edited

Loading