Physics tendency updaters #163

peverwhee · 2021-10-20T19:41:20Z

Updating Externals_CAM and host model to support physics tendency updaters.

Tested with Held-Suarez and no significant differences found using ncdata_check (had to remove qneg from the suite_held_suarez_1994.xml temporarily for testing - qneg updates will come into CAMDEN with future PR).

Addresses #156

…_close_file

gold2718

Questions and suggestions

gold2718 · 2021-10-20T20:38:56Z

src/data/generate_registry_data.py

@@ -433,7 +434,7 @@ def __init__(self, elem_node, parent_name, dimensions, known_types,
                                        ', '.join(dimensions)))
        # end if
        local_name = '{}({})'.format(parent_name, self.index_string)
-        super(ArrayElement, self).__init__(elem_node, local_name, my_dimensions,
+        VarBase.__init__(self, elem_node, local_name, my_dimensions,


I'm not super happy with explicitly calling out the class hierarchy. Not that I have a plan to change this one but changing it is more difficult if we put explicit names in derived classes. Is there a good reason to not use super (here and below)?

I was getting errors using super - I think related to a python version change.

Right. I think this is probably due to taking out the (object) inheritance in the classes.
Since we do not use multiple inheritance, you should be able to just use super() in place of VarBase.

To get that to work, I think I have to change class VarBase to class VarBase(object). Does that seem right?

gold2718 · 2021-10-20T20:44:04Z

src/utils/cam_abortutils.F90

+   private :: active_file_ptr
+


I'm not sure what is going on here but this might be a regression. I think the 'base' version is a fix that @nusbaume put in recently.

good call - reverted.

peverwhee · 2021-10-20T22:25:40Z

@gold2718 @nusbaume Any ideas on how to fix the failing unit tests? It looks like it can't find framework_env.py...?

gold2718

See comment

gold2718 · 2021-10-21T00:07:10Z

src/data/generate_registry_data.py

@@ -433,7 +434,7 @@ def __init__(self, elem_node, parent_name, dimensions, known_types,
                                        ', '.join(dimensions)))
        # end if
        local_name = '{}({})'.format(parent_name, self.index_string)
-        super(ArrayElement, self).__init__(elem_node, local_name, my_dimensions,
+        VarBase.__init__(self, elem_node, local_name, my_dimensions,


Right. I think this is probably due to taking out the (object) inheritance in the classes.
Since we do not use multiple inheritance, you should be able to just use super() in place of VarBase.

nusbaume · 2021-10-21T01:51:07Z

Hi @peverwhee, it looks like framework_env.py doesn't exist anywhere in your CAMDEN branch (including the externals). My guess is that the ccpp-framework external in Externals_CAM.cfg needs updated. Do you know if you updated it at all in your test version, or if you copied in CCPP framework code from somewhere else?

gold2718 · 2021-10-21T03:06:42Z

My guess is that the ccpp-framework external in Externals_CAM.cfg needs updated. Do you know if you updated it at all in your test version, or if you copied in CCPP framework code from somewhere else?

Good catch. It looks like you have CPF_0.2.026, I would try updating it to CPF_0.2.029

nusbaume · 2021-10-25T14:32:06Z

Hi @peverwhee

Just an FYI that to get these unit tests to pass, it looks like you'll need to modify the calls to metadata_file_to_files in test_registry.py so that instead of reading in logging.getLogger("badmf") it uses a CCPPFrameworkEnv object. I believe after this is fixed the unit tests will finally pass, at which point I'll make sure to get through my code review.

Thanks!

peverwhee · 2021-10-25T15:38:26Z

Hi @peverwhee

Just an FYI that to get these unit tests to pass, it looks like you'll need to modify the calls to metadata_file_to_files in test_registry.py so that instead of reading in logging.getLogger("badmf") it uses a CCPPFrameworkEnv object. I believe after this is fixed the unit tests will finally pass, at which point I'll make sure to get through my code review.

Thanks!

Thank you, @nusbaume! I was doing some major flailing.

nusbaume

Looks good, but I do have some questions, and one important request (the phys_timestep_init_zero suggestion).

nusbaume · 2021-10-26T03:02:25Z

cime_config/buildlib

@@ -28,7 +28,7 @@ from CIME.buildlib import parse_input
 from CIME.build import get_standard_makefile_args
 #pylint: enable=wrong-import-position

-check_minimum_python_version(2, 7)
+check_minimum_python_version(3, 7)


Did you test this on Izumi? I know that with our older version of CIME the "default" system version of python is sometimes used (which is python 2.7 on Izumi), but that may only be for the run phase.

You're right - it doesn't work. Not sure how we got it to work before, but right now I'm getting this error during the build stage (despite explicitly loading 3.7.0):

ERROR: Python 3, minor version 7 is required, you have 3.6

@gold2718 any ideas?

Hmm. It seems that the Izumi entry in CIME (<root>/cime/config/cesm/machines/config_machines.xml) is really out of date. It has some python path information that does not exist on Izumi and does not load the python module. Try adding a module load command in the <modules> section of Izumi's entry and see if that helps. If so, open an issue on ESMCI/cime and we can update the entry.

That seems to have fixed the issue! Thank you! In regards to the cime issue, I assume we want to add the module load command for all of the available compilers?

No, just adding it to the <modules> section as a new line after the module purge should do it.

Ahhh that makes sense. Issue created: ESMCI/cime#4116

nusbaume · 2021-10-26T03:03:52Z

cime_config/buildnml

@@ -32,6 +33,8 @@ sys.path.append(_CIME_CONFIG_PATH)
 # Import CAM's configure structure:
 from cam_config import ConfigCAM

+check_minimum_python_version(3, 7)


Does this work on Izumi with our older version of CIME (i.e. can you successfully run case.submit)?

No. Updated cime version and removed this check, and now it runs.

src/data/generate_registry_data.py

src/data/registry.xml

nusbaume · 2021-10-26T03:59:28Z

test/unit/sample_files/write_init_files/simple_build_cache_template.xml

@@ -1 +1 @@
-<CAMBuildCache>TAG1<registry><generate_init_file file_path="$TEST_SOURCE_MODS_DIR/simple_reg.xml" hash="05510a45b99aa9449bb44684362465bf2dacbc6c" />TAG2<generate_registry_file file_path="$TEST_SOURCE_MODS_DIR/simple_reg.xml" hash="05510a45b99aa9449bb44684362465bf2dacbc6c" /><registry_file file_path="$TEST_SOURCE_MODS_DIR/simple_reg.xml" hash="05510a45b99aa9449bb44684362465bf2dacbc6c" /><dycore>none</dycore><config /></registry><CCPP><SDF file_path="$TEST_SOURCE_MODS_DIR/simple_suite.xml" hash="1d90bde78b8740d08cb530748a3ddb346497bf1a" /><scheme file_path="$TEST_SOURCE_MODS_DIR/temp_adjust_scalar.meta" hash="ce34cd13aade63ee06cd82b75170f3c2f6ac1bd5" />TAG3<preproc_defs>UNSET</preproc_defs><kind_phys>REAL64</kind_phys></CCPP></CAMBuildCache>
+<CAMBuildCache>TAG1<registry><generate_init_file file_path="$TEST_SOURCE_MODS_DIR/simple_reg.xml" hash="05510a45b99aa9449bb44684362465bf2dacbc6c" />TAG2<generate_registry_file file_path="$TEST_SOURCE_MODS_DIR/simple_reg.xml" hash="05510a45b99aa9449bb44684362465bf2dacbc6c" /><registry_file file_path="$TEST_SOURCE_MODS_DIR/simple_reg.xml" hash="05510a45b99aa9449bb44684362465bf2dacbc6c" /><dycore>none</dycore><config /></registry><CCPP><SDF file_path="$TEST_SOURCE_MODS_DIR/simple_suite.xml" hash="1d90bde78b8740d08cb530748a3ddb346497bf1a" /><scheme file_path="$TEST_SOURCE_MODS_DIR/temp_adjust_scalar.meta" hash="ca48290fa714873cb0e6cf04968c65d508d6f5dd" />TAG3<preproc_defs>UNSET</preproc_defs><kind_phys>REAL64</kind_phys></CCPP></CAMBuildCache>


Not relevant for this particular PR, but should we pretty print the cache XML file? I realize that a user should never have to look in this file, but I have also had that same thought with a lot of files before and been wrong most of the time.

Any opinions @gold2718 @peverwhee ?

There is a pretty printer class in the CCPP Framework (PrettyElementTree) that you could use (or did I reinvent a wheel here?).
It is probably not in the best place right now (ccpp_datafile.py), I can move it to parse_tools.

pretty printed cache XML file

gold2718

Couple of minor tweaks but then I'm okay to merge.

gold2718 · 2021-10-29T20:47:35Z

cime_config/buildnml

@@ -19,6 +19,7 @@ sys.path.append(_LIBDIR)
 from standard_script_setup          import *
 from CIME.XML.standard_module_setup import *
 from CIME.buildnml                  import create_namelist_infile, parse_input
+from CIME.utils                     import check_minimum_python_version


I guess we do not really need this now.

gold2718 · 2021-10-29T20:50:51Z

src/data/generate_registry_data.py

+        meta_tables = parse_metadata_file(file_path, known_ddts, run_env)
+        # end if


To avoid confusion, these need to be swapped:

Suggested change

meta_tables = parse_metadata_file(file_path, known_ddts, run_env)

# end if

# end if

meta_tables = parse_metadata_file(file_path, known_ddts, run_env)

gold2718 · 2021-10-29T20:53:27Z

src/data/generate_registry_data.py

@@ -1532,6 +1535,10 @@ def write_registry_files(registry, dycore, config, outdir, src_mod, src_root,
    """
    files = []
    known_types = TypeRegistry()
+    logger.debug('hi')


Uhhh, not sure what is going on here. If we need this line, I think the correct syntax is logger.debug('hi mom') ;)

nusbaume

Looks good to me! Just one question related to the addressed issue (which is probably more for @gold2718 anyways).

nusbaume · 2021-11-01T15:39:49Z

cime_config/buildlib

@@ -28,7 +28,7 @@ from CIME.buildlib import parse_input
 from CIME.build import get_standard_makefile_args
 #pylint: enable=wrong-import-position

-check_minimum_python_version(2, 7)
+check_minimum_python_version(3, 6)


Does this mean that for issue #156 we should have the restriction be for a 3.6 minimum (instead of 3.7)?

Does this mean that for issue #156 we should have the restriction be for a 3.6 minimum (instead of 3.7)?

I think so, let's settle this over there (where I have been commenting).

Steve Goldhaber and others added 11 commits September 12, 2021 23:21

Working to add runtime environment

d3a66a3

Implement framework interface changes

fc57eda

Correct standard names for tend variables

4de55c2

Add ic_file_input_names to registry and fix logic bug in cam_register…

caa3f2f

…_close_file

new updaters branch

d394909

merging up

5e26b26

update atmo_phys tag

e6ff6fe

updating registry variable names

72e25d0

updating file variable names

e5b2fd2

updating externals

21a6bf8

fix externals

c004ddd

peverwhee requested review from gold2718 and nusbaume October 20, 2021 19:41

peverwhee changed the title ~~Updaters~~ Physics tendency updaters Oct 20, 2021

gold2718 requested changes Oct 20, 2021

View reviewed changes

Courtney Peverley added 2 commits October 20, 2021 15:04

fix unit tests

7490848

reverting abortutils

349c6e5

gold2718 reviewed Oct 21, 2021

View reviewed changes

Courtney Peverley added 2 commits October 21, 2021 11:01

ccpp_framework externals update; python version specification

9907a1f

update file hashes

e588cf5

fix unit tests

b4e1bc1

nusbaume requested changes Oct 26, 2021

View reviewed changes

Courtney Peverley added 2 commits October 29, 2021 10:09

update atmos_phys tag, pretty print xml

51fbada

update cime external; change python version check to 3.6

7adab47

peverwhee requested a review from gold2718 October 29, 2021 20:30

peverwhee requested a review from nusbaume October 29, 2021 20:31

gold2718 approved these changes Oct 29, 2021

View reviewed changes

address pr comments

b8fe050

nusbaume approved these changes Nov 1, 2021

View reviewed changes

peverwhee merged commit bcef915 into ESCOMP:development Nov 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Physics tendency updaters #163

Physics tendency updaters #163

peverwhee commented Oct 20, 2021 •

edited

Loading

gold2718 left a comment

gold2718 Oct 20, 2021

peverwhee Oct 20, 2021

gold2718 Oct 21, 2021

peverwhee Oct 21, 2021

gold2718 Oct 20, 2021

peverwhee Oct 20, 2021

peverwhee commented Oct 20, 2021

gold2718 left a comment

gold2718 Oct 21, 2021

nusbaume commented Oct 21, 2021 •

edited

Loading

gold2718 commented Oct 21, 2021

nusbaume commented Oct 25, 2021

peverwhee commented Oct 25, 2021

nusbaume left a comment •

edited

Loading

nusbaume Oct 26, 2021

peverwhee Oct 26, 2021

gold2718 Oct 26, 2021

peverwhee Oct 26, 2021

gold2718 Oct 27, 2021

peverwhee Oct 27, 2021

nusbaume Oct 26, 2021

peverwhee Oct 29, 2021

nusbaume Oct 26, 2021

gold2718 Oct 26, 2021

peverwhee Oct 29, 2021

gold2718 left a comment

gold2718 Oct 29, 2021

gold2718 Oct 29, 2021

gold2718 Oct 29, 2021

nusbaume left a comment

nusbaume Nov 1, 2021

gold2718 Nov 1, 2021

		@@ -1 +1 @@
		<CAMBuildCache>TAG1<registry><generate_init_file file_path="$TEST_SOURCE_MODS_DIR/simple_reg.xml" hash="05510a45b99aa9449bb44684362465bf2dacbc6c" />TAG2<generate_registry_file file_path="$TEST_SOURCE_MODS_DIR/simple_reg.xml" hash="05510a45b99aa9449bb44684362465bf2dacbc6c" /><registry_file file_path="$TEST_SOURCE_MODS_DIR/simple_reg.xml" hash="05510a45b99aa9449bb44684362465bf2dacbc6c" /><dycore>none</dycore><config /></registry><CCPP><SDF file_path="$TEST_SOURCE_MODS_DIR/simple_suite.xml" hash="1d90bde78b8740d08cb530748a3ddb346497bf1a" /><scheme file_path="$TEST_SOURCE_MODS_DIR/temp_adjust_scalar.meta" hash="ce34cd13aade63ee06cd82b75170f3c2f6ac1bd5" />TAG3<preproc_defs>UNSET</preproc_defs><kind_phys>REAL64</kind_phys></CCPP></CAMBuildCache>
		<CAMBuildCache>TAG1<registry><generate_init_file file_path="$TEST_SOURCE_MODS_DIR/simple_reg.xml" hash="05510a45b99aa9449bb44684362465bf2dacbc6c" />TAG2<generate_registry_file file_path="$TEST_SOURCE_MODS_DIR/simple_reg.xml" hash="05510a45b99aa9449bb44684362465bf2dacbc6c" /><registry_file file_path="$TEST_SOURCE_MODS_DIR/simple_reg.xml" hash="05510a45b99aa9449bb44684362465bf2dacbc6c" /><dycore>none</dycore><config /></registry><CCPP><SDF file_path="$TEST_SOURCE_MODS_DIR/simple_suite.xml" hash="1d90bde78b8740d08cb530748a3ddb346497bf1a" /><scheme file_path="$TEST_SOURCE_MODS_DIR/temp_adjust_scalar.meta" hash="ca48290fa714873cb0e6cf04968c65d508d6f5dd" />TAG3<preproc_defs>UNSET</preproc_defs><kind_phys>REAL64</kind_phys></CCPP></CAMBuildCache>

		meta_tables = parse_metadata_file(file_path, known_ddts, run_env)
		# end if

Physics tendency updaters #163

Physics tendency updaters #163

Conversation

peverwhee commented Oct 20, 2021 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peverwhee commented Oct 20, 2021

gold2718 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nusbaume commented Oct 21, 2021 • edited Loading

gold2718 commented Oct 21, 2021

nusbaume commented Oct 25, 2021

peverwhee commented Oct 25, 2021

nusbaume left a comment • edited Loading

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nusbaume left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peverwhee commented Oct 20, 2021 •

edited

Loading

nusbaume commented Oct 21, 2021 •

edited

Loading

nusbaume left a comment •

edited

Loading