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.

Sign up for GitHub

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

Jump to bottom

Logging #127

Merged
merged 13 commits into from
Sep 19, 2024
Merged

Logging #127

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
184a614
initial logging for print statements that we added
TeaganKing Aug 29, 2024
b83d8a1
pre commits
TeaganKing Aug 29, 2024
c42f4fc
formatting
TeaganKing Aug 29, 2024
f8db211
remove filename
TeaganKing Sep 4, 2024
f1c599b
include log level in config file and move to utils
TeaganKing Sep 17, 2024
84a41eb
pass logger argument in timeseries and update warnings.warn to logger…
TeaganKing Sep 18, 2024
602af5f
precommit updates
TeaganKing Sep 18, 2024
9e6574d
change default logging level to info since that's probably better whi…
TeaganKing Sep 19, 2024
1383d88
Merge branch 'NCAR:main' into logging
TeaganKing Sep 19, 2024
784086f
updates from review
TeaganKing Sep 19, 2024
72c8cd6
include else statement in case of log level with typo
TeaganKing Sep 19, 2024
889c030
add quotes around log level in config!
TeaganKing Sep 19, 2024
604c5bc
add quotes in coupled_model config
TeaganKing Sep 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cupid/clear.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ def clear(config_path):
Args: CONFIG_PATH - The path to the configuration file.

"""

logger = cupid.util.setup_logging(config_path)
run_dir = read_config_file(config_path)
# Delete the "computed_notebooks" folder and all the contents inside of it
shutil.rmtree(run_dir)
print(f"All contents in {run_dir} have been cleared.")
logger.info(f"All contents in {run_dir} have been cleared.")
11 changes: 6 additions & 5 deletions cupid/run.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
from __future__ import annotations

import os
import warnings

import click
import intake
Expand Down Expand Up @@ -74,6 +73,7 @@ def run(
# Get control structure
control = cupid.util.get_control_dict(config_path)
cupid.util.setup_book(config_path)
logger = cupid.util.setup_logging(config_path)

component_options = {
"atm": atmosphere,
Expand Down Expand Up @@ -180,6 +180,7 @@ def run(
timeseries_params[component]["level"],
num_procs,
serial,
logger,
)
# fmt: on
# pylint: enable=line-too-long
Expand Down Expand Up @@ -245,7 +246,7 @@ def run(
all_nbs[nb]["nb_path_root"] = nb_path_root + "/" + comp_name
all_nbs[nb]["output_dir"] = output_dir + "/" + comp_name
elif comp_bool and not all:
warnings.warn(
logger.warning(
f"No notebooks for {comp_name} component specified in config file.",
)

Expand All @@ -254,7 +255,7 @@ def run(
for nb, info in all_nbs.copy().items():
if not control["env_check"][info["kernel_name"]]:
bad_env = info["kernel_name"]
warnings.warn(
logger.warning(
f"Environment {bad_env} specified for {nb}.ipynb could not be found;" +
f" {nb}.ipynb will not be run." +
"See README.md for environment installation instructions.",
Expand Down Expand Up @@ -288,7 +289,7 @@ def run(
all_scripts[script] = info
all_scripts[script]["nb_path_root"] = nb_path_root + "/" + comp_name
elif comp_bool and not all:
warnings.warn(
logger.warning(
f"No scripts for {comp_name} component specified in config file.",
)

Expand All @@ -297,7 +298,7 @@ def run(
for script, info in all_scripts.copy().items():
if not control["env_check"][info["kernel_name"]]:
bad_env = info["kernel_name"]
warnings.warn(
logger.warning(
f"Environment {bad_env} specified for {script}.py could not be found;" +
f"{script}.py will not be run.",
)
Expand Down
43 changes: 26 additions & 17 deletions cupid/timeseries.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

import xarray as xr

import cupid.util


def call_ncrcat(cmd):
"""This is an internal function to `create_time_series`
Expand All @@ -39,6 +41,7 @@ def create_time_series(
height_dim,
num_procs,
serial,
logger,
):
"""
Generate time series versions of the history file data.
Expand Down Expand Up @@ -85,7 +88,7 @@ def create_time_series(
return

# Notify user that script has started:
print(f"\n Generating {component} time series files...")
logger.info(f"\n Generating {component} time series files...")

# Loop over cases:
for case_idx, case_name in enumerate(case_names):
Expand All @@ -95,11 +98,11 @@ def create_time_series(
"Configuration file indicates time series files have been pre-computed"
)
emsg += f" for case '{case_name}'. Will rely on those files directly."
print(emsg)
logger.info(emsg)
continue
# End if

print(f"\t Processing time series for case '{case_name}' :")
logger.info(f"\t Processing time series for case '{case_name}' :")

# Extract start and end year values:
start_year = start_years[case_idx]
Expand Down Expand Up @@ -184,7 +187,7 @@ def create_time_series(
wmsg += (
f" transferred beyond the {height_dim} dimension itself."
)
print(wmsg)
logger.warning(wmsg)

vert_coord_type = None
# End if
Expand All @@ -195,7 +198,7 @@ def create_time_series(
)
wmsg += " so no additional vertical coordinate information will be"
wmsg += f" transferred beyond the {height_dim} dimension itself."
print(wmsg)
logger.warning(wmsg)

vert_coord_type = None
# End if (long name)
Expand Down Expand Up @@ -228,13 +231,13 @@ def create_time_series(
vars_to_derive = []
# create copy of var list that can be modified for derivable variables
if diag_var_list == ["process_all"]:
print("generating time series for all variables")
logger.info("generating time series for all variables")
# TODO: this does not seem to be working for ocn...
diag_var_list = hist_file_var_list
for var in diag_var_list:
if var not in hist_file_var_list:
if component == "ocn":
print(
logger.warning(
"ocean vars seem to not be present in all files and thus cause errors",
)
continue
Expand All @@ -249,7 +252,7 @@ def create_time_series(
continue
msg = f"WARNING: {var} is not in the file {hist_files[0]}."
msg += " No time series will be generated."
print(msg)
logger.warning(msg)
continue

# Check if variable has a height_dim (eg, 'lev') dimension according to first file:
Expand All @@ -274,7 +277,7 @@ def create_time_series(
continue

# Notify user of new time series file:
print(f"\t - time series for {var}")
logger.info(f"\t - time series for {var}")

# Variable list starts with just the variable
ncrcat_var_list = f"{var}"
Expand All @@ -293,11 +296,11 @@ def create_time_series(

if "PS" in hist_file_var_list:
ncrcat_var_list = ncrcat_var_list + ",PS"
print("Adding PS to file")
logger.info("Adding PS to file")
else:
wmsg = "WARNING: PS not found in history file."
wmsg += " It might be needed at some point."
print(wmsg)
logger.warning(wmsg)
# End if

if vert_coord_type == "height":
Expand All @@ -309,11 +312,11 @@ def create_time_series(
# PMID file to each one of those targets separately. -JN
if "PMID" in hist_file_var_list:
ncrcat_var_list = ncrcat_var_list + ",PMID"
print("Adding PMID to file")
logger.info("Adding PMID to file")
else:
wmsg = "WARNING: PMID not found in history file."
wmsg += " It might be needed at some point."
print(wmsg)
logger.warning(wmsg)
# End if PMID
# End if height
# End if cam
Expand All @@ -335,6 +338,7 @@ def create_time_series(
derive_cam_variables(
vars_to_derive=vars_to_derive,
ts_dir=ts_dir[case_idx],
logger=logger,
)

if serial:
Expand All @@ -347,10 +351,12 @@ def create_time_series(
# End cases loop

# Notify user that script has ended:
print(f" ... {component} time series file generation has finished successfully.")
logger.info(
f" ... {component} time series file generation has finished successfully.",
)


def derive_cam_variables(vars_to_derive=None, ts_dir=None, overwrite=None):
def derive_cam_variables(vars_to_derive=None, ts_dir=None, overwrite=None, logger=None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This function is called from create_time_series(), which uses logger prior to calling in... so I think we want logger to be a required argument. This will let us remove

if logger is None:
    logger = cupid.util.setup_logging("config.yml")

at lines 370-372 (and I think also the import cupid.util at lines 17 & 18)

"""
Derive variables acccording to steps given here. Since derivations will depend on the
variable, each variable to derive will need its own set of steps below.
Expand All @@ -361,6 +367,9 @@ def derive_cam_variables(vars_to_derive=None, ts_dir=None, overwrite=None):
whether to overwrite the file (true) or exit with a warning message.
"""

if logger is None:
logger = cupid.util.setup_logging("config.yml")

for var in vars_to_derive:
if var == "PRECT":
# PRECT can be found by simply adding PRECL and PRECC
Expand All @@ -381,7 +390,7 @@ def derive_cam_variables(vars_to_derive=None, ts_dir=None, overwrite=None):
if overwrite:
Path(prect_file).unlink()
else:
print(
logger.warning(
f"[{__name__}] Warning: PRECT file was found and overwrite is False"
+ "Will use existing file.",
)
Expand Down Expand Up @@ -415,7 +424,7 @@ def derive_cam_variables(vars_to_derive=None, ts_dir=None, overwrite=None):
if overwrite:
Path(derived_file).unlink()
else:
print(
logger.warning(
f"[{__name__}] Warning: RESTOM file was found and overwrite is False."
+ "Will use existing file.",
)
Expand Down
35 changes: 35 additions & 0 deletions cupid/util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"""
from __future__ import annotations

import logging
import os
import sys
import warnings
Expand Down Expand Up @@ -46,6 +47,7 @@ def execute_managed_notebook(cls, nb_man, kernel_name, **kwargs):

def get_control_dict(config_path):
"""Get control dictionary from configuration file"""

try:
with open(config_path) as fid:
control = yaml.safe_load(fid)
Expand Down Expand Up @@ -93,6 +95,39 @@ def get_control_dict(config_path):
return control


def setup_logging(config_path):
"""
Set up logging based on configuration file log level
Returns logger object
"""
control = get_control_dict(config_path)
log_level = control["computation_config"].get("log_level", None)
if log_level:
if log_level == "debug":
logging.basicConfig(
level=logging.DEBUG,
)
if log_level == "info":
logging.basicConfig(
level=logging.INFO,
)
if log_level == "warning":
logging.basicConfig(
level=logging.WARNING,
)
if log_level == "error":
logging.basicConfig(
level=logging.ERROR,
)
else:
# default level is info if log level is not set in config
logging.basicConfig(
level=logging.INFO,
)
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 change line 104 to

log_level = control["computation_config"].get("log_level", "info")

Then you don't need the if statement at 105 or the entire else block from 122 - 126 (maybe move the # default level is info if log level is not set in config comment to above setting log_level?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thinking about this more, should we use

        if log_level == "debug":
            logging.basicConfig(
                level=logging.DEBUG,
            )
        elif log_level == "info":
            logging.basicConfig(
                level=logging.INFO,
            )
        elif log_level == "warning":
            logging.basicConfig(
                level=logging.WARNING,
            )
        elif log_level == "error":
            logging.basicConfig(
                level=logging.ERROR,
            )
        else:

and trap the case when log_level is not one of the four expected strings?


return logging.getLogger(__name__)


def setup_book(config_path):
"""Setup run directory and output jupyter book"""

Expand Down
5 changes: 4 additions & 1 deletion examples/coupled_model/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,12 @@ computation_config:
### It must already be installed on your machine. You can also
### specify a different environment than the default for any
### notebook in NOTEBOOK CONFIG

default_kernel_name: cupid-analysis

# log level sets the level of how verbose logging will be.
# options include: debug, info, warning, error
log_level: info

############# NOTEBOOK CONFIG #############

############################
Expand Down
5 changes: 4 additions & 1 deletion examples/key_metrics/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,12 @@ computation_config:
### It must already be installed on your machine. You can also
### specify a different environment than the default for any
### notebook in NOTEBOOK CONFIG

default_kernel_name: cupid-analysis

# log level sets the level of how verbose logging will be.
# options include: debug, info, warning, error
log_level: info

############# NOTEBOOK CONFIG #############

############################
Expand Down
Loading