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

Derived variables structure update #326

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

justin-richling
Copy link
Collaborator

Move the calculation of derived variables out of adf_daig.py into an external script. This is a placeholder until a more robust variable derivations scheme is implemented.

This will help declutter adf_diag.py in the hopes of keeping non-essential methods separate.

adf_derive.py has two main methods: check_derive and derive_variable

  • check_derive - called in variable loop; will check on each available variable
    For incoming variable, look for list of constituents if available as a list in variable defaults yaml file

    If the variable does not have the argument derivable_from or derivable_from_cam_chem,
    then it will be assumed not to be a derivable variable, just missing from history file

    If the variable does have the argument derivable_from or derivable_from_cam_chem,
    first check cam-chem, then regular cam and add all constituents to diag_var_list for time series generation.

    NOTE: this will only modify diag_var_list for use in adf_diag.py, it will not modify the yaml variable list used in other scripts.

  • derive_variable - called after variable loop and time series generation; loop over dictionary containing derived variables as keys and list of constituents as values.
    This will then be called in a loop of accepted derived variables and will be generated from constituent time series files.

Pull these checks and calculations out of `adf-diag.py` to clean that file up.
Now call the `adf_derive.py` script for derived variables
Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup! Just have a few suggestions.

lib/adf_derive.py Show resolved Hide resolved
lib/adf_derive.py Show resolved Hide resolved
Comment on lines +103 to +104
else:
self.debug_log(constit_errmsg)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of checking whether constit_list is a list type below, would it possibly be safer to just add a logical that skips writing the constit_errmsg if you have already written it here?

# End if

# Log if this variable can be derived but is missing list of constituents
if isinstance(constit_list, list) and not constit_list:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use the logical method suggested above, then you can probably get rid of the isinstance check here.

lib/adf_derive.py Show resolved Hide resolved
lib/adf_derive.py Show resolved Hide resolved
lib/adf_derive.py Show resolved Hide resolved
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.

2 participants