Clean up PFT-related data structures and migrate k-loop out of interior_tendency_compute() #318
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added a constructor to the type to allocate memory, and also moved the local variable from marbl_interior_tendency_compute() to marbl_interface_class.
Added a constructor to the type to allocate memory, and also moved the local variable from marbl_interior_tendency_compute() to marbl_interface_class. Similar to 69384dd, but with a different structure.
One instance where autotroph_local was being passed with the wrong intent.
compute_autotroph_elemental_ratios, compute_function_scaling, and compute_Pprime are outside the k loop in interior_tendency_compute(). compute_autotroph_elemental_ratios has its own k-loop that I need to clean up, but the other two routines act on vectors.
* autotroph_secondary_species_type -> autotroph_derived_terms_type * zooplankton_secondary_species_type -> zooplankton_derived_terms_type * autotroph_type -> autotroph_settings_type (and autotrophs -> autotroph_settings)
Biggest changes are * autotrophs -> autotroph_settings * zooplankton -> zooplankton_settings * grazing -> grazer_settings Those three require YAML (and JSON) changes, and then python modifications due to the changes in the JSON. Plus updates to the POP python wrappers. This commit is still bit-for-bit in POP testing, but all tests will now fail the namelist comparison due to changes in the variable names.
|
This pull request is not ready to be merged yet, but I want
|
I had inadvertently changed the _CESM2_PFT_keys options in the cesm2.0 default settings file (wanted to keep autotrophs and zooplankton but change grazing -> grazer, accidentally added _settings for all of them). Caught by the POP test trying to use the old settings - could also have seen the issue running MARBL_generate_settings_file.py with the cesm2.0 default file, but that isn't part of any automated testing.
instead of passing domain and autotroph_derived_terms, just pass the values we need out of those structures (km, zt, and Pprime)
Use vector arithmetic instead of computing these ratios level by level in an explicit loop. (Still passing km to the routine so we can explicitly declare array lengths.)
Talking to Keith, removing the k-loop in favor of vector arithmetic isn't useful given the ordering of the dimensions. I've put the k-loop back in, though I find it more readable to have the k-loop inside the associate statement.
Also, explicitly declare dimensions of arrays being passed.
|
Address #158 with |
Also, explicitly declare dimensions of arrays being passed
compute_autotroph_photosynthesis does not need VNtot from autotroph_derived_terms.
compute_autotroph_nutrient_uptake() is a more accurate name
Added a constructor to the type to allocate memory and moved the local variable from marbl_interior_tendency_compute() onto marbl_interface_class. Similar to 5105a79
Call this subroutine immediately after compute_particulate_terms(); when we push k-loop into the latter, will also roll in the call to the former.
Also, explicitly declare dimensions of arrays being passed
Added a constructor to the type to allocate memory and moved the local variable from marbl_interior_tendency_compute() onto marbl_interface_class. Similar to lots of prior commits, including the last one.
particulate_share, surface_flux_share, and surface_flux_internal are constructed in marbl_interface%init() rather than in marbl_init_*(). Further, surface_flux_share (and interior_tendency_share) only need to be constructed / destructed when ciso_on is true. Note that particulate_share is used for bury coefficients regardless of the state of ciso_on (so perhaps it needs to be renamed).
Pull marbl_interior_tendency_share_export_variables from k loop
Added a constructor to the type to allocate memory and moved the local variable from marbl_interior_tendency_compute onto marbl_interface_class. Similar to lots of prior commits, most recently bd130b4.
Pull marbl_interior_tendency_share_export_zooplankton from k loop
Take two: this time, associate with PAR%interface(0,:) to avoid issues with array indexing (this commit is still bit-for-bit, whereas 33eb778 changed answers)
This is a better name for the variable, as it pertains to a predator / prey pair rather than simply the grazer (i.e. the predator). Also, I cleaned up some of the logic behind setting up the PFT keys (like "((grazer_sname))") in MARBL_settings_file_class.py
Switch to vector notation now that vector dimensions have been pulled into data types.
|
6a4b02e is bit-for-bit with |
loops should be auto_ind first and k second. Also, by making zero_mask an array of length column_kmt, we can reduce the calls to the marbl_ciso consistency_enforce routine to once per autotroph instead of once per level per autotroph.
marbl_ciso_interior_tendency_autotroph_zero_consistency_enforce only needs a single element of marbl_tracer_indices%auto_inds(:), not the entire array.
Loop over auto_ind, use vector notation for k dimension (since it's all contiguous memory)
Given memory layout, loop over k inside auto_ind loop rather than vice versa for compute_autotroph_photosynthesis()
Loop over auto_ind, use vector notation for k dimension (since it's all contiguous memory)
Loop over auto_ind, use vector notation for k dimension (since it's all contiguous memory)
Loop over auto_ind, use vector notation for k dimension (since it's all contiguous memory)
Loop over auto_ind, use vector notation for k dimension
Loop over auto_ind, use vector notation for k dimension
Loop over zoo_ind, use vector notation for k dimension. Also, compute f_loss_thres with min(max(..., c0), c1) rather than where statements (in both compute_Zprime and compute_Pprime, which had been vectorized in a previous commit)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Many data structures had names that were not descriptive, so it was confusing what the purpose of the structures were. Also, there were several datatypes that were declared as arrays when it made more sense to have the array dimensions inside the datatype.
These changes go hand-in-hand with moving the
kloop out ofinterior_tendency_compute()-- moving the array dimensions inside the type will make it easier to vectorize computations across a single column rather than needing to loop over each level.This pull request is restricted to being bit-for-bit, so there will likely be additional vector-based optimizations to make at a later date.
Addresses #53 (perhaps not entirely) and #317. The Fortran interface will not change, but the names of variables in the settings file will - that will affect the GCMs and also require documentation updates.