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

Enhancement/generalize saved state #64

Merged
merged 8 commits into from
May 23, 2016
Merged

Enhancement/generalize saved state #64

merged 8 commits into from
May 23, 2016

Conversation

mnlevy1981
Copy link
Collaborator

This branch generalizes the saved state datatype. It requires

https://svn-ccsm-models.cgd.ucar.edu/pop2/branch_tags/marbl_dev_levy_tags/marbl_dev_levy_n19_marbl_dev_n10_cesm_pop_2_1_20160513

and passes all aux_pop_MARBL tests (yellowstone_intel, yellowstone_gnu, and hobart_nag).

@klindsay28 I'm hoping we can talk about this on Friday or Monday.

I added two saved state arrays to marbl_interface_class -- one for surface
forcing and one for interior forcing. There are also two corresponding index
types. Two major changes I see needing to happen:
1. I hardcoded the number '2' for how many saved state variables there are (ph
   and ph_alt_co2) in both the surface and interior types. I think the optimal
   setup would be similar to surface forcing output, where the arrays are
   "re-allocatable".
2. I think we want a marbl_saved_state_mod to house some of the initialization
   routines, rather than calling add_saved_state directly from
   marbl_instance%init.
I was adding fields to the surface forcing saved state rather than interior,
which was resulting in out of memory errors. Also, I wasn't processing the log
output after returning from "add to saved state" calls.
Currently only support rank 2 and rank 3 saved state variables, and rank has to
be compatible with vgrid ('none' for rank=2, and either 'layer_avg' or
'layer_iface' for rank=3). I hope this allows us to add rank=0 for global sums,
but I'm not sure exactly what that will look like since MARBL will need to pass
out the full 2D field so the GCM can do the area-weighting for computation.
Forgot a pretty important line in the last commit...
@mnlevy1981
Copy link
Collaborator Author

Looking through the code diffs after commit fbb3be2, there is still a little more work to be done. I'll continue to clean this code up and let you know when it's actually ready to be reviewed.

@mnlevy1981
Copy link
Collaborator Author

I'm in the process of setting up a marbl_saved_state_mod, and I've noticed a slight discrepancy in how the code is organized. marbl_log_type is defined in marbl_logging.F90, despite being part of the interface - I would have expected this datatype to be in marbl_interface_types.F90. Not sure what the best thing to do is, but it's something to pay attention to when we reach the code-cleanup stage of the project.

Created marbl_saved_state_mod.F90, currently only contains
marbl_saved_state_init but more routines may be added down the line.
Mimicking the surface forcing output type, saved state uses pointers to
"reallocate" memory when adding a new saved state variable.
@mnlevy1981
Copy link
Collaborator Author

I think this branch is ready for review. I probably need better comments in marbl_saved_state_mod.F90 but I like how the code itself looks - the generalized saved_state type initializes in a similar manner to the surface forcing output type: it allocates memory for each state%add() call, rather than requiring a hard count of the number of saved state fields or cycling through the fields twice (once to count, once to actually set up the data type).

@mnlevy1981
Copy link
Collaborator Author

mnlevy1981 commented May 19, 2016

Forgot to note that a2baa5e passes aux_pop_MARBL and is bit-for-bit with latest trunk on yellowstone (intel and gnu) as well as hobart (nag)

* The saved state indexing types are now in marbl_internal.F90
* field_2d and field_3d in single_saved_state_type have a comment indicating
  the proper dimensions (in the data type definition)
* Consistent spacing in saved_state_mod
@mnlevy1981
Copy link
Collaborator Author

9274328 using

https://svn-ccsm-models.cgd.ucar.edu/pop2/branch_tags/marbl_dev_levy_tags/marbl_dev_levy_n20_marbl_dev_n10_cesm_pop_2_1_20160513

passes all tests and is bit-for-bit with baselines (aux_pop on yellowstone_intel, aux_pop_marbl on yellowstone_intel, yellowstone_gnu, and hobart_nag)

@mnlevy1981 mnlevy1981 merged commit 9274328 into marbl-ecosys:master May 23, 2016
@mnlevy1981 mnlevy1981 deleted the enhancement/generalize_saved_state branch May 24, 2016 02:39
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.

1 participant