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

Code clean-up in new tracer indices #59

Open
mnlevy1981 opened this issue May 7, 2016 · 1 comment
Open

Code clean-up in new tracer indices #59

mnlevy1981 opened this issue May 7, 2016 · 1 comment
Assignees
Milestone

Comments

@mnlevy1981
Copy link
Collaborator

mnlevy1981 commented May 7, 2016

The main issue is that we want the tracer count comparison (OGCM vs MARBL) to be done in the driver, rather than MARBL. However, I have a few other small things that snuck through the code review...

  • The consistency check that is currently in the routine marbl_mod:marbl_check_ecosys_tracer_count_consistency should be moved to part of the tracer index constructor routine (similar CISO consistency check is already there)
  • The documentation of the tracer index type reads
    ! This subroutine sets the tracer indices for the non-autotroph tracers. To
    ! know where to start the indexing for the autotroph tracers, it increments
    ! tracer_cnt by 1 for each tracer that is included. Note that this gives an
    ! accurate count whether the carbon isotope tracers are included or not.

That is inaccurate, it sets tracer indices for ALL tracers.

  • In both the marbl_tracer_index_type definition and the constructor, comments refer to General tracers instead of base ecosys tracers (we have been using ecosys_base for tracking indices)
  • Relatedly, maybe we want tracer_module_name = 'base' instead of tracer_module_name = 'ecosys'
@mnlevy1981 mnlevy1981 self-assigned this May 7, 2016
@mnlevy1981 mnlevy1981 added this to the Feature-complete API milestone May 7, 2016
@mnlevy1981
Copy link
Collaborator Author

In my initial description I suggested

The consistency check that is currently in the routine marbl_mod:marbl_check_ecosys_tracer_count_consistency should be moved to part of the tracer index constructor routine (similar CISO consistency check is already there)

Instead, we have moved the CISO tracer index consistency check out of the indexing type.

However, the other three points still need addressing:

  1. marbl_internal_types:tracer_index_constructor description still mentions non-autotroph tracers instead of all tracers
  2. Base ecosystem class tracers are still referred to as "general" in both the marbl_tracer_index_type class definition and comments in marbl_internal_types:tracer_index_constructor
  3. marbl_mod:marbl_init_tracer_metadata still sets tracer_module_name to 'ecosys' instead of 'base'

Also, we have nine MARBL classes used to track various indices. marbl_tracer_index_type and marbl_living_tracer_index_type are in the minority by ending in index_type; the other seven (surface_forcing_diagnostics, surface_forcing, surface_forcing_output, surface_saved_state, interior_diagnostics, interior_forcing, and interior_saved_state) end with indexing_type and I think we should be consistent here. Speaking of consistency, surface_forcing_diagnostics vs interior_diagnostics...

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

1 participant