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.

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

Updates from mom6 driver #419

Merged

Conversation

mnlevy1981
Copy link
Collaborator

The big feature is generalizing the surface_flux_output_type so MARBL can provide the GCM with fields from interior_tendency_compute() as well as surface_flux_compute(); this new functionality is used to allow the option of returning the 3D chlorophyll field (POP only needed the surface chlorophyll, but MOM needs the 3D field).

I also added an option to check consistency in the PAR forcing field, but it is turned off by default because there is a huge performance hit.

Warn the GCM if the sum of the ice category fractions is not close to 1
(threshold is hard-coded to be within 1e-6 of 1)
Adding check_forcing() dropped throughput of a 1-mo C test case from 9 SYPD to
2.3 SYPD... so I think we want the default to be not to call this function.
Renamed a few data types:

* marbl_surface_flux_output_type -> marbl_output_for_GCM_type
* marbl_single_sfo_type -> marbl_single_output_type

Also replaced "totalChl" with "total_surfChl" in the indices / constructor
because we will use total_Chl for the 3D field.

marbl_single_output_type now contains both forcing_field_0d and
forcing_field_1d, because total_Chl will need to allocate memory with
num_levels. I removed num_elements from the marbl_output_for_GCM_type because
it was never used.

There was lots of renaming variables to replace sfo or surface_flux_output with
something more generic.
I think this is everything we need to pass 3D Chl back to the GCM
When copying from this%outputs_for_GCM to new_output before deallocating
this%outputs_for_GCM / reassigning the pointer, I had forgotten to copy
field_1d. This revealed itself as a problem in CI, because we check for unused
variables and I had declared dim2_loc in anticipation of re-allocating field_1d
call_compute_subroutines test adds three more fields to history file:

output_for_GCM_flux_co2
output_for_GCM_total_surfChl
output_for_GCM_total_Chl

This commit will fail CI because the baseline file does not contain these
fields; next commit will contain updated baseline.
Baseline file now includes the three new fields added to the output
MOM6 needs to know the initial total_Chl field, so waiting until we call
interior_tendency_compute() is not sufficient. So rather than getting totalChl
in an analagous manner to how POP gets surface Chl, there is a new function on
the interface [get_output_for_GCM()] that takes the tracer array and an integer
index indicating what field to return as an intent(in).

Note that this new function replaces negative tracer values with 0, but does
not enforce consistency among the autotroph tracers (so Chl may be positive
while C, P, or Fe are 0). Because of this, I expect this commit to fail CI but
I'll update the baseline in the next commit and everything should pass again.
Also removed an unused variable declaration and added a fill value to the
output_for_GCM_total_Chl field in the netCDF file since it is undefined below
the ocean floor.
Rather than passing in an array of tracer values, users need to make sure
this%tracers has the latest values (we were hoping that passing in an array of
tracers would avoid a copy in the MOM6 driver, but the way tracers are stored
in a derived type meant we were copying it anyway)
@mnlevy1981 mnlevy1981 linked an issue Feb 23, 2023 that may be closed by this pull request
@mnlevy1981 mnlevy1981 marked this pull request as ready for review March 7, 2023 23:08
Right now we are only using get_output_for_GCM() to return 1D fields, but we
may want to introduce an interface to allow it to return scalars as well
First version set num_levels = -1 for 2D fields, but 0 makes more sense
Removed the following:

1. use statement for a type that is not used in the module
2. return statement inside a function that doesn't need to be there
3. comment refering to a block of code that was removed earlier in this
   development

Also cleaned up one last comment that refered to surface flux output instead of
output for the GCM.
@mnlevy1981
Copy link
Collaborator Author

@klindsay28 and I reviewed this PR on March 8th; 0c46cd5, 1828803, and 75852f1 address all the small changes. The only outstanding item from the code review is that check_forcing() is triggering a ton of warnings in a C compset, and the additional output to stdout is a significant hindrance to performance. Before this is merged, we need to figure out what is going wrong with the C compset forcing; once those warnings are reduced I'll have one more commit to this PR where I remove the lcheck_forcing logical flag (we'll check the forcing every time)

@mnlevy1981
Copy link
Collaborator Author

Per further discussion with @klindsay28, we're going to merge this as is and address the problems in check_forcing() separately.

@mnlevy1981 mnlevy1981 merged commit 5a41872 into marbl-ecosys:development Mar 22, 2023
@mnlevy1981 mnlevy1981 deleted the updates_from_mom6_driver branch March 22, 2023 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generalize marbl_surface_flux_output_type
1 participant