Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Made MOM_control_struct opaque #701

Merged
merged 14 commits into from
Feb 3, 2018
Merged

Made MOM_control_struct opaque #701

merged 14 commits into from
Feb 3, 2018

Conversation

Hallberg-NOAA
Copy link
Collaborator

Updated MOM6_examples to use a series of commits that eliminate the direct use
of elements of the MOM_control_struct at the driver level, and introduce a
shared MOM_state_type. This set of commits include changes to the order of
entries in the MOM_parameter_doc files. In a note of caution, this sequence of
commit includes changes to mct_driver/ocn_comp_mct.F90 that mirror changes to
coupled_driver/ocean_model_MOM.F90, but have not been tested even to the point
of compiling due to the unavailability at GFDL of various software used by this
coupler; help is needed from someone who uses the mct_coupler to verify that
these have been done correctly!

@Hallberg-NOAA
+Created a MOM_state_type
3cd24bc 
  Created a new MOM_state_type and separated it from the MOM_control_structure,
including adding new parallel arguments to many of the routines in MOM.F90.
This change also required changes to the various coupled and ocean-only MOM
drivers to reflect elements of these public types that had moved.  All answers
are bitwise identical.
@Hallberg-NOAA
Merge branch 'dev/gfdl' into restructure_MOM_CS
7b42449
@Hallberg-NOAA
+Reduced use of MOM_CSp elements by drivers
4747b1d 
  Moved t_dyn_rel_adv and ave_ssh from the MOM_control_structure to the
MOM_state_type. Added an optional argument to ocean_model_init to set a pointer
to the diag_ctrl type, and then used this to avoid using MOM_CSp%diag in the
top-level drivers.  Also avoided using MOM%CSp%use_temperature by querying
whether tv%T is associated.  All answers are bitwise identical, but there are
several changes to interfaces and types.
@Hallberg-NOAA
+Moved restart_CS up to the driver level
a1bcc6f 
  Moved the primary location of the restart_CS out of the MOM control structure
and into the top-level MOM driver, since that is the level where it is actually
used.  This changes the interface to initialize_MOM and it changes the elements
of several control structures.  All solutions are bitwise identical.
@Hallberg-NOAA
+Moved ENERGYSAVEDAYS into MOM_sum_output.F90
78031f9 
  Moved the code that determines the ocean.stats output frequency into
MOM_sum_output, which allows the code that is duplicated between the various
drivers to be eliminated. Also added the runtime parameter WRITE_CPU_STEPS,
which determines how often MOM checks CPU time in ocean-only runs.  All answers
are bitwise identical, but the entries and order of entries in the
MOM_parameter_doc files change.
@Hallberg-NOAA
Merge branch 'dev/gfdl' into restructure_MOM_CS
add27a3
@Hallberg-NOAA
Generate fatal error with excessive energy or NaNs
dd8fdd6 
  Changed the behavior of MOM_sum_output when NaNs or excessive energy per unit
mass are detected to give a fatal error, rather than resetting the time to a
huge value as before.  This now replicates the error handling with an excessive
number of velocity truncations. All answers are bitwise identical, although the
ocean-only model behavior changes during model failures.
@Hallberg-NOAA
+Moved write_energy into step_MOM
93656d4 
  Moved all calls to write_energy, MOM_sum_output_init, and accumulate_net_input
into MOM.F90 and out of the various MOM drivers.  All answers are bitwise
identical, but there are changes to public types and a new optional argument
(count_calls) has been added to initialize_MOM.
@Hallberg-NOAA
+Made MOM_control_struct opaque
b8c358e 
  Eliminated the external use of elements from the MOM_control_struct in the
driver routines, and made the MOM_control_struct private. Doing so also involved
adding a new optional argument to initialize_MOM returning a pointer to the
tracer_flow_control_CS for use by the driver in initializing the tracer surface
forcing.  All answers are bitwise identical, but the change in types is not
backward compatible and requires the changes to the top-level drivers.
gustavo-marques
gustavo-marques reviewed Jan 29, 2018
View reviewed changes
Copy link
Collaborator

@gustavo-marques gustavo-marques left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Hallberg-NOAA , thanks for adding these changes to mct_driver/ocn_comp_mct.F90. I am unable to compile CESM/MOM6 after these commits. Below are the error messages:

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(840): error #6404: This name does not have a type, and must have an explicit type. [USE_TEMPERATURE]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(840): error #6303: The assignment operation or the binary expression operation is invalid for the data types of the two operands. [ASSOCIATED]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(910): error #6633: The type of the actual argument differs from the type of the dummy argument. [FLUXES]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1731): error #6633: The type of the actual argument differs from the type of the dummy argument. [FLUXES]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1731): error #6633: The type of the actual argument differs from the type of the dummy argument. [TIME]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1731): error #6633: The type of the actual argument differs from the type of the dummy argument. [TIME_STEP]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1731): error #6633: The type of the actual argument differs from the type of the dummy argument. [ICE_SHELF_CSP]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1731): error #6631: A non-optional actual argument must be present when invoking a procedure with an explicit interface. [CS]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1751): error #6633: The type of the actual argument differs from the type of the dummy argument. [FLUX_TMP]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1751): error #6633: The type of the actual argument differs from the type of the dummy argument. [TIME]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1751): error #6633: The type of the actual argument differs from the type of the dummy argument. [TIME_STEP]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1751): error #6633: The type of the actual argument differs from the type of the dummy argument. [ICE_SHELF_CSP]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1751): error #6631: A non-optional actual argument must be present when invoking a procedure with an explicit interface. [CS]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1761): error #6633: The type of the actual argument differs from the type of the dummy argument. [FLUXES]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1761): error #6633: The type of the actual argument differs from the type of the dummy argument. [TIME_STEP]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1761): error #6633: The type of the actual argument differs from the type of the dummy argument. [GRID]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1761): error #6633: The type of the actual argument differs from the type of the dummy argument. [WEIGHT]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1761): error #6631: A non-optional actual argument must be present when invoking a procedure with an explicit interface. [WT2]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1776): error #6633: The type of the actual argument differs from the type of the dummy argument. [FLUXES]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1776): error #6633: The type of the actual argument differs from the type of the dummy argument. [STATE]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1776): error #6633: The type of the actual argument differs from the type of the dummy argument. [TIME1]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1776): error #6633: The type of the actual argument differs from the type of the dummy argument. [TIME_STEP]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1776): error #6633: The type of the actual argument differs from the type of the dummy argument. [MSP]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1776): error #6633: The type of the actual argument differs from the type of the dummy argument. [MOM_CSP]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1776): error #6631: A non-optional actual argument must be present when invoking a procedure with an explicit interface. [CS]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1778): error #6633: The type of the actual argument differs from the type of the dummy argument. [FLUXES]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1778): error #6633: The type of the actual argument differs from the type of the dummy argument. [STATE]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1778): error #6633: The type of the actual argument differs from the type of the dummy argument. [TIME1]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1778): error #6633: The type of the actual argument differs from the type of the dummy argument. [TIME_STEP]

/glade/p/work/gmarques/cesm2_0_beta06/components/mom/src/MOM6/config_src/mct_driver/ocn_comp_mct.F90(1778): error #6633: The type of the actual argument differs from the type of the dummy argument. [MSP]

@kshedstrom
Copy link
Collaborator

kshedstrom commented Jan 29, 2018 via email

@gustavo-marques
Copy link
Collaborator

Thanks @kshedstrom for mentioning that, but I don't think this is why I am getting these errors. I have FMS version 34097d0, which is what dev/gfdl MOM-example is pointing to. If I was using a wrong FMS version, errors would also occur in other modules, no just mct_driver/ocn_comp_mct.F90. I can compile/run the model using the dev/ncar branch, which is in sync with the latest dev/master.

@Hallberg-NOAA
Copy link
Collaborator Author

The compilation errors that @gustavo-marques are finding are not due FMS problems. They are due to the fact that I had to make changes to the mct_driver code to accommodate interface changes to MOM.F90, but I do not have the ability to compile that driver due to software packages we are missing here at GFDL. As noted in the original PR message, these compile-time errors are not a surprise, which is why @gustavo-marques was specifically asked to review them. My suggestion is that @gustavo-marques and I take this discussion off-line and come up with a corrected version.

This set of changes to the coupler / ocean interface should be a one-time occurrence, but going forward we also need to figure out a better strategy for testing all coupled configurations.

This was referenced Jan 31, 2018
Closed
Merged
@Hallberg-NOAA
Removed the other trailing blanks in a comment
eee3738
@Hallberg-NOAA
Merge branch 'dev/gfdl' into restructure_MOM_CS
b4454d4
@Hallberg-NOAA
Corrected tracer diagnostic metadata formatting
6680003 
  Corrected formatting of longname and units metadata for some newly added
tracer tendency diagnostics.  All answers are bitwise identical, but there are
minor changes to the available_diags files.
@Hallberg-NOAA
Merge branch 'dev/gfdl' into restructure_MOM_CS
30d2757
@Hallberg-NOAA
Copy link
Collaborator Author

The code conflicts were resolved with the merge of the latest version of dev/gfdl into the code for this PR, and the version of mct_driver/ocn_comp_mct.F90 that was merged in is almost identical to one has been tested by @gustavo-marques, so this should now be a viable candidate for merging onto dev/gfdl .

@adcroft adcroft merged commit 30d2757 into mom-ocean:dev/gfdl Feb 3, 2018
@Hallberg-NOAA Hallberg-NOAA deleted the restructure_MOM_CS branch May 7, 2018 15:37
OlgaSergienko pushed a commit to OlgaSergienko/MOM6 that referenced this pull request Aug 5, 2024
@Wendazhang33
Fix bugs in setting GxSpV_u and GxSpV_v in MOM_isopycnal_slopes (mom-…
e30a6e7 
…ocean#701)

* Fix bugs in setting GxSpV_u and GxSpV_v in MOM_isopycnal_slopes
 - In the previous version, GxSpV_u and GxSpV_v were only set when use_EOS was true
 - Now initialize GxSpV_u and GxSpV_v to be G_Rho0

* Fixed an OMP issue in MOM_isopycnal_slopes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gustavo-marques gustavo-marques gustavo-marques left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Hallberg-NOAA @kshedstrom @gustavo-marques @adcroft