Skip to content

Commit

Permalink
Remove dependence on root directory
Browse files Browse the repository at this point in the history
This change forces paths to be relative to the CWD throughout the code
base unless absolute paths are required.

Improves code readability and the logging to standard output.

Fixes #162
  • Loading branch information
SeanBryan51 committed Dec 1, 2023
1 parent ded19e5 commit d37b36b
Show file tree
Hide file tree
Showing 10 changed files with 94 additions and 187 deletions.
13 changes: 4 additions & 9 deletions benchcab/benchcab.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
class Benchcab:
"""A class that represents the `benchcab` application."""

root_dir: Path = internal.CWD
subprocess_handler: SubprocessWrapperInterface = SubprocessWrapper()
modules_handler: EnvironmentModulesInterface = EnvironmentModules()

Expand Down Expand Up @@ -143,10 +142,10 @@ def fluxsite_submit_job(
msg = "Path to benchcab executable is undefined."
raise RuntimeError(msg)

job_script_path = self.root_dir / internal.QSUB_FNAME
job_script_path = Path(internal.QSUB_FNAME)

Check warning on line 145 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L145

Added line #L145 was not covered by tests
print(
"Creating PBS job script to run fluxsite tasks on compute "
f"nodes: {job_script_path.relative_to(self.root_dir)}"
f"nodes: {job_script_path}"
)
with job_script_path.open("w", encoding="utf-8") as file:
contents = render_job_script(
Expand Down Expand Up @@ -202,12 +201,8 @@ def checkout(self, config_path: str, verbose: bool):
)
cable_aux_repo.checkout(verbose=verbose)

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

Check warning on line 205 in benchcab/benchcab.py

View check run for this annotation

Codecov / codecov/patch

benchcab/benchcab.py#L204-L205

Added lines #L204 - L205 were not covered by tests
with rev_number_log_path.open("w", encoding="utf-8") as file:
file.write(rev_number_log)

Expand Down
5 changes: 1 addition & 4 deletions benchcab/comparison.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
class ComparisonTask:
"""A class used to represent a single bitwise comparison task."""

root_dir: Path = internal.CWD
subprocess_handler: SubprocessWrapperInterface = SubprocessWrapper()

def __init__(
Expand All @@ -42,9 +41,7 @@ def run(self, verbose=False) -> None:
print(f"Success: files {file_a.name} {file_b.name} are identical")
except CalledProcessError as exc:
output_file = (
self.root_dir
/ internal.FLUXSITE_DIRS["BITWISE_CMP"]
/ f"{self.task_name}.txt"
internal.FLUXSITE_DIRS["BITWISE_CMP"] / f"{self.task_name}.txt"
)
with output_file.open("w", encoding="utf-8") as file:
file.write(exc.stdout)
Expand Down
73 changes: 24 additions & 49 deletions benchcab/fluxsite.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ class CableError(Exception):
class Task:
"""A class used to represent a single fluxsite task."""

root_dir: Path = internal.CWD
subprocess_handler: SubprocessWrapperInterface = SubprocessWrapper()

def __init__(
Expand Down Expand Up @@ -147,10 +146,7 @@ def setup_task(self, verbose=False):
self.fetch_files(verbose=verbose)

nml_path = (
self.root_dir
/ internal.FLUXSITE_DIRS["TASKS"]
/ self.get_task_name()
/ internal.CABLE_NML
internal.FLUXSITE_DIRS["TASKS"] / self.get_task_name() / internal.CABLE_NML
)

if verbose:
Expand All @@ -162,25 +158,26 @@ def setup_task(self, verbose=False):
"filename": {
"met": str(internal.MET_DIR / self.met_forcing_file),
"out": str(
self.root_dir
/ internal.FLUXSITE_DIRS["OUTPUT"]
/ self.get_output_filename()
(
internal.FLUXSITE_DIRS["OUTPUT"]
/ self.get_output_filename()
).resolve()
),
"log": str(
self.root_dir
/ internal.FLUXSITE_DIRS["LOG"]
/ self.get_log_filename()
(
internal.FLUXSITE_DIRS["LOG"] / self.get_log_filename()
).resolve()
),
"restart_out": " ",
"type": str(self.root_dir / internal.GRID_FILE),
"type": str(internal.GRID_FILE.resolve()),
},
"output": {
"restart": False,
},
"fixedCO2": internal.CABLE_FIXED_CO2_CONC,
"casafile": {
"phen": str(self.root_dir / internal.PHEN_FILE),
"cnpbiome": str(self.root_dir / internal.CNPBIOME_FILE),
"phen": str(internal.PHEN_FILE.resolve()),
"cnpbiome": str(internal.CNPBIOME_FILE.resolve()),
},
"spinup": False,
}
Expand Down Expand Up @@ -210,9 +207,7 @@ def clean_task(self, verbose=False):
if verbose:
print(" Cleaning task")

task_dir = (
self.root_dir / internal.FLUXSITE_DIRS["TASKS"] / self.get_task_name()
)
task_dir = internal.FLUXSITE_DIRS["TASKS"] / self.get_task_name()

cable_exe = task_dir / internal.CABLE_EXE
if cable_exe.exists():
Expand All @@ -230,17 +225,11 @@ def clean_task(self, verbose=False):
if cable_soil_nml.exists():
cable_soil_nml.unlink()

output_file = (
self.root_dir
/ internal.FLUXSITE_DIRS["OUTPUT"]
/ self.get_output_filename()
)
output_file = internal.FLUXSITE_DIRS["OUTPUT"] / self.get_output_filename()
if output_file.exists():
output_file.unlink()

log_file = (
self.root_dir / internal.FLUXSITE_DIRS["LOG"] / self.get_log_filename()
)
log_file = internal.FLUXSITE_DIRS["LOG"] / self.get_log_filename()
if log_file.exists():
log_file.unlink()

Expand All @@ -253,19 +242,14 @@ def fetch_files(self, verbose=False):
- copies contents of 'namelists' directory to 'runs/fluxsite/tasks/<task_name>' directory.
- copies cable executable from source to 'runs/fluxsite/tasks/<task_name>' directory.
"""
task_dir = (
self.root_dir / internal.FLUXSITE_DIRS["TASKS"] / self.get_task_name()
)
task_dir = internal.FLUXSITE_DIRS["TASKS"] / self.get_task_name()

if verbose:
print(
f" Copying namelist files from {self.root_dir / internal.NAMELIST_DIR} "
f"to {task_dir}"
f" Copying namelist files from {internal.NAMELIST_DIR} to {task_dir}"
)

shutil.copytree(
self.root_dir / internal.NAMELIST_DIR, task_dir, dirs_exist_ok=True
)
shutil.copytree(internal.NAMELIST_DIR, task_dir, dirs_exist_ok=True)

exe_src = self.model.get_exe_path()
exe_dest = task_dir / internal.CABLE_EXE
Expand All @@ -280,7 +264,7 @@ def fetch_files(self, verbose=False):
def run(self, verbose=False):
"""Runs a single fluxsite task."""
task_name = self.get_task_name()
task_dir = self.root_dir / internal.FLUXSITE_DIRS["TASKS"] / task_name
task_dir = internal.FLUXSITE_DIRS["TASKS"] / task_name

Check warning on line 267 in benchcab/fluxsite.py

View check run for this annotation

Codecov / codecov/patch

benchcab/fluxsite.py#L267

Added line #L267 was not covered by tests
if verbose:
print(
f"Running task {task_name}... CABLE standard output "
Expand All @@ -303,14 +287,14 @@ def run_cable(self, verbose=False):
Raises `CableError` when CABLE returns a non-zero exit code.
"""
task_name = self.get_task_name()
task_dir = self.root_dir / internal.FLUXSITE_DIRS["TASKS"] / task_name
task_dir = internal.FLUXSITE_DIRS["TASKS"] / task_name
stdout_path = task_dir / internal.CABLE_STDOUT_FILENAME

try:
with chdir(task_dir):
self.subprocess_handler.run_cmd(
f"./{internal.CABLE_EXE} {internal.CABLE_NML}",
output_file=stdout_path,
output_file=stdout_path.relative_to(task_dir),
verbose=verbose,
)
except CalledProcessError as exc:
Expand All @@ -323,16 +307,9 @@ def add_provenance_info(self, verbose=False):
Attributes include branch url, branch revision number and key value pairs in
the namelist file used to run cable.
"""
nc_output_path = (
self.root_dir
/ internal.FLUXSITE_DIRS["OUTPUT"]
/ self.get_output_filename()
)
nc_output_path = internal.FLUXSITE_DIRS["OUTPUT"] / self.get_output_filename()
nml = f90nml.read(
self.root_dir
/ internal.FLUXSITE_DIRS["TASKS"]
/ self.get_task_name()
/ internal.CABLE_NML
internal.FLUXSITE_DIRS["TASKS"] / self.get_task_name() / internal.CABLE_NML
)
if verbose:
print(f"Adding attributes to output file: {nc_output_path}")
Expand Down Expand Up @@ -389,16 +366,14 @@ def run_tasks_in_parallel(
pool.map(run_task, tasks, chunksize=1)


def get_fluxsite_comparisons(
tasks: list[Task], root_dir=internal.CWD
) -> list[ComparisonTask]:
def get_fluxsite_comparisons(tasks: list[Task]) -> list[ComparisonTask]:
"""Returns a list of `ComparisonTask` objects to run comparisons with.
Pairs should be matching in science configurations and meteorological
forcing, but differ in realisations. When multiple realisations are
specified, return all pair wise combinations between all realisations.
"""
output_dir = root_dir / internal.FLUXSITE_DIRS["OUTPUT"]
output_dir = internal.FLUXSITE_DIRS["OUTPUT"]
return [
ComparisonTask(
files=(
Expand Down
3 changes: 2 additions & 1 deletion benchcab/internal.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@
)

# Fluxsite directory tree
FLUXSITE_DIRS = {}
FLUXSITE_DIRS: dict[str, Path] = {}

# Relative path to root directory for CABLE fluxsite runs
FLUXSITE_DIRS["RUN"] = RUN_DIR / "fluxsite"

Expand Down
51 changes: 16 additions & 35 deletions benchcab/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
class Model:
"""A class used to represent a CABLE model version."""

root_dir: Path = internal.CWD
subprocess_handler: SubprocessWrapperInterface = SubprocessWrapper()
modules_handler: EnvironmentModulesInterface = EnvironmentModules()

Expand Down Expand Up @@ -61,19 +60,12 @@ def model_id(self, value: int):
def get_exe_path(self) -> Path:
"""Return the path to the built executable."""
return (
self.root_dir
/ internal.SRC_DIR
/ self.name
/ self.src_dir
/ "offline"
/ internal.CABLE_EXE
internal.SRC_DIR / self.name / self.src_dir / "offline" / internal.CABLE_EXE
)

def custom_build(self, modules: list[str], verbose=False):
"""Build CABLE using a custom build script."""
build_script_path = (
self.root_dir / internal.SRC_DIR / self.name / self.build_script
)
build_script_path = internal.SRC_DIR / self.name / self.build_script

if not build_script_path.is_file():
msg = (
Expand All @@ -85,6 +77,7 @@ def custom_build(self, modules: list[str], verbose=False):

tmp_script_path = build_script_path.parent / "tmp-build.sh"

# TODO(Sean) should this be a utility function?
if verbose:
print(f"Copying {build_script_path} to {tmp_script_path}")
shutil.copy(build_script_path, tmp_script_path)
Expand All @@ -110,50 +103,40 @@ def custom_build(self, modules: list[str], verbose=False):

def pre_build(self, verbose=False):
"""Runs CABLE pre-build steps."""
path_to_repo = self.root_dir / internal.SRC_DIR / self.name
path_to_repo = internal.SRC_DIR / self.name
tmp_dir = path_to_repo / self.src_dir / "offline" / ".tmp"
if not tmp_dir.exists():
if verbose:
print(f"mkdir {tmp_dir.relative_to(self.root_dir)}")
print(f"mkdir {tmp_dir}")
tmp_dir.mkdir()

for pattern in internal.OFFLINE_SOURCE_FILES:
for path in (path_to_repo / self.src_dir).glob(pattern):
if not path.is_file():
continue
copy2(
path.relative_to(self.root_dir),
tmp_dir.relative_to(self.root_dir),
verbose=verbose,
)
copy2(path, tmp_dir, verbose=verbose)

copy2(
(path_to_repo / self.src_dir / "offline" / "Makefile").relative_to(
self.root_dir
),
tmp_dir.relative_to(self.root_dir),
(path_to_repo / self.src_dir / "offline" / "Makefile"),
tmp_dir,
verbose=verbose,
)

copy2(
(path_to_repo / self.src_dir / "offline" / "parallel_cable").relative_to(
self.root_dir
),
tmp_dir.relative_to(self.root_dir),
(path_to_repo / self.src_dir / "offline" / "parallel_cable"),
tmp_dir,
verbose=verbose,
)

copy2(
(path_to_repo / self.src_dir / "offline" / "serial_cable").relative_to(
self.root_dir
),
tmp_dir.relative_to(self.root_dir),
(path_to_repo / self.src_dir / "offline" / "serial_cable"),
tmp_dir,
verbose=verbose,
)

def run_build(self, modules: list[str], verbose=False):
"""Runs CABLE build scripts."""
path_to_repo = self.root_dir / internal.SRC_DIR / self.name
path_to_repo = internal.SRC_DIR / self.name
tmp_dir = path_to_repo / self.src_dir / "offline" / ".tmp"

with chdir(tmp_dir), self.modules_handler.load(modules, verbose=verbose):
Expand All @@ -177,14 +160,12 @@ def run_build(self, modules: list[str], verbose=False):

def post_build(self, verbose=False):
"""Runs CABLE post-build steps."""
path_to_repo = self.root_dir / internal.SRC_DIR / self.name
path_to_repo = internal.SRC_DIR / self.name
tmp_dir = path_to_repo / self.src_dir / "offline" / ".tmp"

rename(
(tmp_dir / internal.CABLE_EXE).relative_to(self.root_dir),
(path_to_repo / self.src_dir / "offline" / internal.CABLE_EXE).relative_to(
self.root_dir
),
(tmp_dir / internal.CABLE_EXE),
(path_to_repo / self.src_dir / "offline" / internal.CABLE_EXE),
verbose=verbose,
)

Expand Down
4 changes: 2 additions & 2 deletions benchcab/utils/fs.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def copy2(src: Path, dest: Path, verbose=False):
shutil.copy2(src, dest)


def next_path(path: Path, path_pattern: str, sep: str = "-"):
def next_path(path_pattern: str, path: Path = Path(), sep: str = "-") -> Path:
"""Finds the next free path in a sequentially named list of
files with the following pattern in the `path` directory:
Expand All @@ -54,7 +54,7 @@ def next_path(path: Path, path_pattern: str, sep: str = "-"):
common_filename, last_file_index = pattern_files_sorted[-1].stem.split(sep)
new_file_index = int(last_file_index) + 1

return f"{common_filename}{sep}{new_file_index}{loc_pattern.suffix}"
return Path(f"{common_filename}{sep}{new_file_index}{loc_pattern.suffix}")


def mkdir(new_path: Path, verbose=False, **kwargs):
Expand Down
Loading

0 comments on commit d37b36b

Please sign in to comment.