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

103 we should be using the logging module #224

Merged
merged 19 commits into from
Feb 15, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
2f6da74
Added baseline logging implementation, tests relying on stdout need t…
bschroeter Dec 18, 2023
59e56fe
Reorganised logger, updated most debugs to infos. Fixes #103
bschroeter Dec 20, 2023
43deac8
Added logger instantiation to CLI. Fixes #103
bschroeter Dec 20, 2023
63b41f8
Automatic application of ruff/black suggestions. Manual edits to foll…
bschroeter Dec 21, 2023
023c4d4
Manual ruff modifications. #103
bschroeter Dec 21, 2023
ff36e53
Manual black modifications. #103
bschroeter Dec 21, 2023
d26937b
Merge branch 'main' into 103-we-should-be-using-the-logging-module
bschroeter Jan 19, 2024
7686403
Refactoring of verbosity
bschroeter Jan 24, 2024
222ad43
Black reformatting. #103
bschroeter Jan 24, 2024
1ba25b0
Reverted to native logger singleton, less flexible but pickles. #103
bschroeter Feb 6, 2024
7d84532
Applied black/ruff checks, ran integration tests successfully. Fixes …
bschroeter Feb 6, 2024
f618128
Merge update from main. Fixes #103
bschroeter Feb 6, 2024
44818a4
Updated again from main - further commits coming to handle clobbered …
bschroeter Feb 12, 2024
222d16d
Ruff checks, pytest passes. #103.
bschroeter Feb 13, 2024
9b49c13
Black reapplied. #103
bschroeter Feb 13, 2024
7a3b741
Merge branch 'main' into 103-we-should-be-using-the-logging-module
abhaasgoyal Feb 14, 2024
b6bca5b
Remaining clobbered code fix. #103.
bschroeter Feb 14, 2024
3957aaf
Merge branch '103-we-should-be-using-the-logging-module' of github.co…
bschroeter Feb 14, 2024
b65ddbb
Last clobbered merge changes.
Feb 15, 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
6 changes: 5 additions & 1 deletion benchcab/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@

import importlib.metadata

from benchcab.utils import get_logger

try:
__version__ = importlib.metadata.version("benchcab")
except importlib.metadata.PackageNotFoundError:
__version__ = ""
print("Warning: unable to interrogate version string from installed distribution.")
get_logger().warn(
"unable to interrogate version string from installed distribution."
)
# Note: cannot re-raise exception here as this will break pytest
# when running without first installing the package
198 changes: 113 additions & 85 deletions benchcab/benchcab.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
)
from benchcab.internal import get_met_forcing_file_names
from benchcab.model import Model
from benchcab.utils import get_logger
from benchcab.utils.fs import mkdir, next_path
from benchcab.utils.pbs import render_job_script
from benchcab.utils.repo import SVNRepo, create_repo
Expand All @@ -41,42 +42,56 @@
benchcab_exe_path: Optional[Path],
validate_env: bool = True,
) -> None:
"""Constructor.

Parameters
----------
benchcab_exe_path : Optional[Path]
Path to the executable.
validate_env : bool, optional
Validate the environment, by default True
"""
self.benchcab_exe_path = benchcab_exe_path
self.validate_env = validate_env

self._config: Optional[dict] = None
self._models: list[Model] = []
self.tasks: list[Task] = [] # initialise fluxsite tasks lazily

# Get the logger object
self.logger = get_logger()

def _validate_environment(self, project: str, modules: list):
"""Performs checks on current user environment."""
if not self.validate_env:
return

if "gadi.nci" not in internal.NODENAME:
print("Error: benchcab is currently implemented only on Gadi")
self.logger.error("benchcab is currently implemented only on Gadi")

Check warning on line 70 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L70

Added line #L70 was not covered by tests
sys.exit(1)

namelist_dir = Path(internal.CWD / internal.NAMELIST_DIR)
if not namelist_dir.exists():
print(
"Error: cannot find 'namelists' directory in current working directory"
self.logger.error(

Check warning on line 75 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L75

Added line #L75 was not covered by tests
"Cannot find 'namelists' directory in current working directory"
)
sys.exit(1)

required_groups = [project, "ks32", "hh5"]
groups = [grp.getgrgid(gid).gr_name for gid in os.getgroups()]
if not set(required_groups).issubset(groups):
print(
"Error: user does not have the required group permissions.",
"The required groups are:",
", ".join(required_groups),
self.logger.error(

Check warning on line 83 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L83

Added line #L83 was not covered by tests
[
"User does not have the required group permissions.",
"The required groups are:",
" ,".join(required_groups),
]
)
sys.exit(1)

for modname in modules:
if not self.modules_handler.module_is_avail(modname):
print(f"Error: module ({modname}) is not available.")
self.logger.error(f"Module ({modname}) is not available.")

Check warning on line 94 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L94

Added line #L94 was not covered by tests
sys.exit(1)

all_site_ids = set(
Expand All @@ -86,14 +101,16 @@
for site_id in all_site_ids:
paths = list(internal.MET_DIR.glob(f"{site_id}*"))
if not paths:
print(
f"Error: failed to infer met file for site id '{site_id}' in "
f"{internal.MET_DIR}."
self.logger.error(

Check warning on line 104 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L104

Added line #L104 was not covered by tests
[
f"Failed to infer met file for site id '{site_id}' in "
f"{internal.MET_DIR}."
]
)
sys.exit(1)
if len(paths) > 1:
print(
f"Error: multiple paths infered for site id: '{site_id}' in {internal.MET_DIR}."
self.logger.error(

Check warning on line 112 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L112

Added line #L112 was not covered by tests
f"Multiple paths infered for site id: '{site_id}' in {internal.MET_DIR}."
)
sys.exit(1)

Expand Down Expand Up @@ -124,13 +141,11 @@
)
return self.tasks

def validate_config(self, config_path: str, verbose: bool):
def validate_config(self, config_path: str):
"""Endpoint for `benchcab validate_config`."""
_ = self._get_config(config_path)

def fluxsite_submit_job(
self, config_path: str, verbose: bool, skip: list[str]
) -> None:
def fluxsite_submit_job(self, config_path: str, skip: list[str]) -> None:
"""Submits the PBS job script step in the fluxsite test workflow."""
config = self._get_config(config_path)
self._validate_environment(project=config["project"], modules=config["modules"])
Expand All @@ -139,17 +154,17 @@
raise RuntimeError(msg)

job_script_path = Path(internal.QSUB_FNAME)
print(
"Creating PBS job script to run fluxsite tasks on compute "
f"nodes: {job_script_path}"
self.logger.info(

Check warning on line 157 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L157

Added line #L157 was not covered by tests
"Creating PBS job script to run fluxsite tasks on compute nodes"
)

self.logger.info(f"job_script_path = {job_script_path}")

Check warning on line 161 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L161

Added line #L161 was not covered by tests

with job_script_path.open("w", encoding="utf-8") as file:
contents = render_job_script(
project=config["project"],
config_path=config_path,
modules=config["modules"],
pbs_config=config["fluxsite"]["pbs"],
verbose=verbose,
Copy link
Member

Choose a reason for hiding this comment

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

We want to keep the pbs_config argument here! That comes from #238

skip_bitwise_cmp="fluxsite-bitwise-cmp" in skip,
benchcab_path=str(self.benchcab_exe_path),
)
Expand All @@ -159,94 +174,102 @@
proc = self.subprocess_handler.run_cmd(
f"qsub {job_script_path}",
capture_output=True,
verbose=verbose,
)
except CalledProcessError as exc:
print("Error when submitting job to NCI queue")
print(exc.output)
self.logger.error("when submitting job to NCI queue, details to follow")
self.logger.error(exc.output)

Check warning on line 180 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L179-L180

Added lines #L179 - L180 were not covered by tests
raise

print(
f"PBS job submitted: {proc.stdout.strip()}\n"
"The CABLE log file for each task is written to "
f"{internal.FLUXSITE_DIRS['LOG']}/<task_name>_log.txt\n"
"The CABLE standard output for each task is written to "
f"{internal.FLUXSITE_DIRS['TASKS']}/<task_name>/out.txt\n"
"The NetCDF output for each task is written to "
f"{internal.FLUXSITE_DIRS['OUTPUT']}/<task_name>_out.nc"
)
self.logger.info(f"PBS job submitted: {proc.stdout.strip()}")
self.logger.info("CABLE log file for each task is written to:")
self.logger.info(f"{internal.FLUXSITE_DIRS['LOG']}/<task_name>_log.txt")
self.logger.info("The CABLE standard output for each task is written to:")
self.logger.info(f"{internal.FLUXSITE_DIRS['TASKS']}/<task_name>/out.txt")
self.logger.info("The NetCDF output for each task is written to:")
self.logger.info(f"{internal.FLUXSITE_DIRS['OUTPUT']}/<task_name>_out.nc")

Check warning on line 189 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L183-L189

Added lines #L183 - L189 were not covered by tests

def checkout(self, config_path: str, verbose: bool):
def checkout(self, config_path: str):
"""Endpoint for `benchcab checkout`."""
config = self._get_config(config_path)
self._validate_environment(project=config["project"], modules=config["modules"])

mkdir(internal.SRC_DIR, exist_ok=True, verbose=True)
mkdir(internal.SRC_DIR, exist_ok=True)

Check warning on line 196 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L196

Added line #L196 was not covered by tests

print("Checking out repositories...")
self.logger.info("Checking out repositories...")

Check warning on line 198 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L198

Added line #L198 was not covered by tests
rev_number_log = ""
for model in self._get_models(config):
model.repo.checkout(verbose=verbose)
model.repo.checkout()

Check warning on line 201 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L201

Added line #L201 was not covered by tests
rev_number_log += f"{model.name}: {model.repo.get_revision()}\n"

# TODO(Sean) we should archive revision numbers for CABLE-AUX
cable_aux_repo = SVNRepo(

Check warning on line 205 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L205

Added line #L205 was not covered by tests
svn_root=internal.CABLE_SVN_ROOT,
branch_path=internal.CABLE_AUX_RELATIVE_SVN_PATH,
path=internal.SRC_DIR / "CABLE-AUX",
)
cable_aux_repo.checkout(verbose=verbose)

Check warning on line 210 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L210

Added line #L210 was not covered by tests
Copy link
Member

Choose a reason for hiding this comment

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

We removed this in #230


rev_number_log_path = next_path("rev_number-*.log")
print(f"Writing revision number info to {rev_number_log_path}")
self.logger.info(f"Writing revision number info to {rev_number_log_path}")

Check warning on line 213 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L213

Added line #L213 was not covered by tests
with rev_number_log_path.open("w", encoding="utf-8") as file:
file.write(rev_number_log)

print("")

def build(self, config_path: str, verbose: bool):
def build(self, config_path: str):
"""Endpoint for `benchcab build`."""
config = self._get_config(config_path)
self._validate_environment(project=config["project"], modules=config["modules"])

for repo in self._get_models(config):
if repo.build_script:
print(
"Compiling CABLE using custom build script for "
f"realisation {repo.name}..."
)
repo.custom_build(modules=config["modules"], verbose=verbose)

self.logger.info("Compiling CABLE using custom build script for")
self.logger.info(f"realisation {repo.name}")
repo.custom_build(modules=config["modules"])

Check warning on line 227 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L225-L227

Added lines #L225 - L227 were not covered by tests

else:
build_mode = "with MPI" if internal.MPI else "serially"
print(f"Compiling CABLE {build_mode} for realisation {repo.name}...")
repo.pre_build(verbose=verbose)
repo.run_build(modules=config["modules"], verbose=verbose)
repo.post_build(verbose=verbose)
print(f"Successfully compiled CABLE for realisation {repo.name}")
print("")

def fluxsite_setup_work_directory(self, config_path: str, verbose: bool):
self.logger.info(

Check warning on line 231 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L231

Added line #L231 was not covered by tests
f"Compiling CABLE {build_mode} for realisation {repo.name}..."
)
repo.pre_build()
repo.run_build(modules=config["modules"])
repo.post_build()
self.logger.info(f"Successfully compiled CABLE for realisation {repo.name}")

Check warning on line 237 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L234-L237

Added lines #L234 - L237 were not covered by tests

def fluxsite_setup_work_directory(self, config_path: str):
"""Endpoint for `benchcab fluxsite-setup-work-dir`."""
config = self._get_config(config_path)
self._validate_environment(project=config["project"], modules=config["modules"])

tasks = self.tasks if self.tasks else self._initialise_tasks(config)
print("Setting up run directory tree for fluxsite tests...")
setup_fluxsite_directory_tree(verbose=verbose)
print("Setting up tasks...")
self.logger.info("Setting up run directory tree for fluxsite tests...")
setup_fluxsite_directory_tree()
self.logger.info("Setting up tasks...")

Check warning on line 247 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L245-L247

Added lines #L245 - L247 were not covered by tests
for task in tasks:
task.setup_task(verbose=verbose)
print("Successfully setup fluxsite tasks")
print("")
task.setup_task()
self.logger.info("Successfully setup fluxsite tasks")

Check warning on line 250 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L249-L250

Added lines #L249 - L250 were not covered by tests

def fluxsite_run_tasks(self, config_path: str, verbose: bool):
def fluxsite_run_tasks(self, config_path: str):
"""Endpoint for `benchcab fluxsite-run-tasks`."""
config = self._get_config(config_path)
self._validate_environment(project=config["project"], modules=config["modules"])

tasks = self.tasks if self.tasks else self._initialise_tasks(config)
print("Running fluxsite tasks...")
if config["fluxsite"]["multiprocess"]:
ncpus = config["fluxsite"]["pbs"]["ncpus"]
try:
multiprocess = config["fluxsite"]["multiprocess"]
except KeyError:
multiprocess = internal.FLUXSITE_DEFAULT_MULTIPROCESS
if multiprocess:
ncpus = config.get("pbs", {}).get(

Check warning on line 264 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L259-L264

Added lines #L259 - L264 were not covered by tests
"ncpus", internal.FLUXSITE_DEFAULT_PBS["ncpus"]
)
run_tasks_in_parallel(tasks, n_processes=ncpus, verbose=verbose)
else:
run_tasks(tasks, verbose=verbose)
print("Successfully ran fluxsite tasks")
print("")
run_tasks(tasks)
self.logger.info("Successfully ran fluxsite tasks")

Check warning on line 270 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L269-L270

Added lines #L269 - L270 were not covered by tests
Copy link
Member

Choose a reason for hiding this comment

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

We want the version in main here but I believe the print at the top should call the logger.


def fluxsite_bitwise_cmp(self, config_path: str, verbose: bool):
def fluxsite_bitwise_cmp(self, config_path: str):
"""Endpoint for `benchcab fluxsite-bitwise-cmp`."""
config = self._get_config(config_path)
self._validate_environment(project=config["project"], modules=config["modules"])
Expand All @@ -260,31 +283,36 @@
comparisons = get_fluxsite_comparisons(tasks)

print("Running comparison tasks...")
if config["fluxsite"]["multiprocess"]:
ncpus = config["fluxsite"]["pbs"]["ncpus"]
try:
multiprocess = config["fluxsite"]["multiprocess"]
except KeyError:
multiprocess = internal.FLUXSITE_DEFAULT_MULTIPROCESS
if multiprocess:
try:
ncpus = config["fluxsite"]["pbs"]["ncpus"]
except KeyError:
ncpus = internal.FLUXSITE_DEFAULT_PBS["ncpus"]

Check warning on line 294 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L286-L294

Added lines #L286 - L294 were not covered by tests
Copy link
Member

Choose a reason for hiding this comment

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

We want the version in main. And we want to call self.logger.info instead of print on the first line.

run_comparisons_in_parallel(comparisons, n_processes=ncpus, verbose=verbose)
else:
run_comparisons(comparisons, verbose=verbose)
print("Successfully ran comparison tasks")
run_comparisons(comparisons)
self.logger.info("Successfully ran comparison tasks")

Check warning on line 298 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L297-L298

Added lines #L297 - L298 were not covered by tests

def fluxsite(
self, config_path: str, no_submit: bool, verbose: bool, skip: list[str]
):
def fluxsite(self, config_path: str, no_submit: bool, skip: list[str]):
"""Endpoint for `benchcab fluxsite`."""
self.checkout(config_path, verbose)
self.build(config_path, verbose)
self.fluxsite_setup_work_directory(config_path, verbose)
self.checkout(config_path)
self.build(config_path)
self.fluxsite_setup_work_directory(config_path)

Check warning on line 304 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L302-L304

Added lines #L302 - L304 were not covered by tests
if no_submit:
self.fluxsite_run_tasks(config_path, verbose)
self.fluxsite_run_tasks(config_path)

Check warning on line 306 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L306

Added line #L306 was not covered by tests
if "fluxsite-bitwise-cmp" not in skip:
self.fluxsite_bitwise_cmp(config_path, verbose)
self.fluxsite_bitwise_cmp(config_path)

Check warning on line 308 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L308

Added line #L308 was not covered by tests
else:
self.fluxsite_submit_job(config_path, verbose, skip)
self.fluxsite_submit_job(config_path, skip)

Check warning on line 310 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L310

Added line #L310 was not covered by tests

def spatial(self, config_path: str, verbose: bool):
def spatial(self, config_path: str):
"""Endpoint for `benchcab spatial`."""

def run(self, config_path: str, no_submit: bool, verbose: bool, skip: list[str]):
def run(self, config_path: str, no_submit: bool, skip: list[str]):
"""Endpoint for `benchcab run`."""
self.fluxsite(config_path, no_submit, verbose, skip)
self.spatial(config_path, verbose)
self.fluxsite(config_path, no_submit, skip)
self.spatial(config_path)

Check warning on line 318 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L317-L318

Added lines #L317 - L318 were not covered by tests
Loading
Loading