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

Generalize saved_state API #49

Closed
mnlevy1981 opened this issue Apr 21, 2016 · 5 comments
Closed

Generalize saved_state API #49

mnlevy1981 opened this issue Apr 21, 2016 · 5 comments

Comments

@mnlevy1981
Copy link
Collaborator

Right now, marbl_saved_state_type looks like

  type, public :: marbl_saved_state_type
     real (r8), allocatable :: ph_prev_col(:)          ! (km)
     real (r8), allocatable :: ph_prev_alt_co2_col(:)  ! (km)
     real (r8), allocatable :: ph_prev_surf(:)         ! (num_elements)
     real (r8), allocatable :: ph_prev_alt_co2_surf(:) ! (num_elements)
   contains
     procedure, public :: construct => marbl_saved_state_constructor
  end type marbl_saved_state_type

We need to generalize this in a way similar to what we have done with the diagnostics - remove the hard-coded field names in favor of something where we can easily add new variables that need to be saved from time step to time step when needed. This lets the driver allocate memory at runtime for the saved state variables, which has two main benefits:

  1. Additional saved state fields can be added in MARBL without requiring any updates to the driver
  2. The driver does not need to know which fields are being saved from time-step to time-step, so this removes some un-necessary semantic content from the driver.
    * Note that this has one drawback: the GCM will write a generic "saved_state" field to restart files, so we may want to include a way for MARBL to verify that the indexing has not changed from one run to the next.
@mnlevy1981 mnlevy1981 self-assigned this Apr 21, 2016
@mnlevy1981 mnlevy1981 modified the milestone: Prototype MARBL API Apr 22, 2016
@mnlevy1981 mnlevy1981 removed the ready label May 2, 2016
@mnlevy1981
Copy link
Collaborator Author

running_mean_mod handles allocating an array of io_desc which we will want to mimic in write_restart()

@mnlevy1981
Copy link
Collaborator Author

@klindsay28 discussed this earlier today, and an interesting conundrum came up. The purpose of saved state is to provide a placeholder for variables that need to be held from one time step to the next, whether from set_surface_forcing or set_interior. POP requires us to call set_surface_forcing before set_interior, so we would also need to use saved state to pass data from the latter to the former (because it crosses a time step). Variables like dust_flux, which are passed from the surface forcing to the interior, don't need to be in saved state in POP but may need to be saved in GCMs that call the interior first?

While talking about dust_flux we realized that we should see what comes out of #56 before making final decisions. This issue is still my current priority, but future code-refactoring may be required.

@mnlevy1981
Copy link
Collaborator Author

Also, I was going to look and see if PH_SURF is just the top level of PH_3D in saved state; if so, maybe we don't need two separate fields for PH? @klindsay28 says he thinks the two were inconsistent in past versions of the code, but might be the same now.

@klindsay28
Copy link
Collaborator

It looks like ph_3d an ph_surf use different initial bounding intervals for the carbonate solver (e.g. phlo_surf_init is different from phlo_3d_init). So I would not expect ph_3d and ph_surf to identical in model output (or restart files). But I think it would OK to unify them. Note that I'm not saying we should, only that this current difference is not imperative.

@mnlevy1981
Copy link
Collaborator Author

Fixed with #64

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants