Skip to content

Define a more strictly typed "sim_cfg" in some Deploy subclasses #153

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

Open
rswarbrick wants to merge 1 commit into
base: master
Choose a base branch
from
+18 −7
Open

Define a more strictly typed "sim_cfg" in some Deploy subclasses #153

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 18 additions & 7 deletions src/dvsim/job/deploy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from dvsim.job.time import JobTime
from dvsim.logging import log
from dvsim.report.data import IPMeta, ToolMeta
from dvsim.test import Test
from dvsim.tool.utils import get_sim_tool_plugin
from dvsim.utils import (
clean_odirs,
Expand Down Expand Up @@ -372,10 +373,14 @@ class CompileSim(Deploy):
cmds_list_vars: ClassVar = ["pre_build_cmds", "post_build_cmds"]
weight = 5

def __init__(self, build_mode, sim_cfg) -> None:
def __init__(self, build_mode: "BuildMode", sim_cfg: "SimCfg") -> None:
"""Initialise a Sim compile stage job deployment."""
self.build_mode_obj = build_mode
self.seed = sim_cfg.build_seed

# Register a copy of sim_cfg which is explicitly the SimCfg type
self._typed_sim_cfg: SimCfg = sim_cfg
Comment on lines +381 to +382
Copy link
Copy Markdown
Contributor

@AlexJones0 AlexJones0 Apr 2, 2026

Choose a reason for hiding this comment

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

This seems reasonable - my personal preference for python is to use from __future__ import annotations and (long-term) to decouple the objects to resolve circular dependencies and fix these kinds of issues, but I guess we haven't done this for historical reasons (using older Python versions). This is a pragmatic workaround for the meantime.

Referring back to my comment on #144:

I wonder, would it not be a good idea to rename sim_cfg -> flow_cfg as well?

I think my preference would be to have the base Deploy etc. take a flow_cfg: "FlowCfg" and then have these deploys that only work for simulation flows take a sim_cfg: SimCfg. That way we can just have self.flow_cfg and perhaps self._sim_cfg instead of self._typed_sim_cfg. With the same explanatory comment above, I think that would probably help with readability.

Even better would be to use a property so it is inherently clear that one is derived from the other and we don't have duplicated references, but I think that would run into the same circular import issue for now.


super().__init__(sim_cfg)

# Needs to be after the wildcard expansion to log anything meaningful
Expand Down Expand Up @@ -439,7 +444,7 @@ def _set_attrs(self) -> None:
# 'build_mode' is used as a substitution variable in the HJson.
self.build_mode = self.name
self.job_name += f"_{self.build_mode}"
if self.sim_cfg.cov:
if self._typed_sim_cfg.cov:
self.output_dirs += [self.cov_db_dir]
self.pass_patterns = self.build_pass_patterns
self.fail_patterns = self.build_fail_patterns
Expand Down Expand Up @@ -542,7 +547,10 @@ class RunTest(Deploy):
fixed_seed = None
cmds_list_vars = ["pre_run_cmds", "post_run_cmds"]

def __init__(self, index, test, build_job, sim_cfg: "SimCfg") -> None:
def __init__(self, index: int, test: Test, build_job: CompileSim, sim_cfg: "SimCfg") -> None:
# Register a copy of sim_cfg which is explicitly the SimCfg type
self._typed_sim_cfg: SimCfg = sim_cfg

self.test_obj = test
self.index = index
self.build_seed = sim_cfg.build_seed
Expand All @@ -567,7 +575,7 @@ def __init__(self, index, test, build_job, sim_cfg: "SimCfg") -> None:
self.run_timeout_mins,
)

if build_job is not None and not self.sim_cfg.run_only:
if build_job is not None and not self._typed_sim_cfg.run_only:
self.dependencies.append(build_job)

# We did something wrong if build_mode is not the same as the build_job
Expand Down Expand Up @@ -621,7 +629,7 @@ def _set_attrs(self) -> None:
self.qual_name = self.run_dir_name + "." + str(self.seed)
self.full_name = f"{self.sim_cfg.name}{self._variant_suffix}:{self.qual_name}"
self.job_name += f"_{self.build_mode}"
if self.sim_cfg.cov:
if self._typed_sim_cfg.cov:
self.output_dirs += [self.cov_db_dir]

# In GUI mode, the log file is not updated; hence, nothing to check.
Expand Down Expand Up @@ -797,8 +805,11 @@ class CovReport(Deploy):
target = "cov_report"
weight = 10

def __init__(self, merge_job, sim_cfg) -> None:
def __init__(self, merge_job: CovMerge, sim_cfg: "SimCfg") -> None:
"""Initialise a job deployment to generate a coverage report."""
# Register a copy of sim_cfg which is explicitly the SimCfg type
self._typed_sim_cfg: SimCfg = sim_cfg

super().__init__(sim_cfg)
self.dependencies.append(merge_job)

Expand Down Expand Up @@ -835,7 +846,7 @@ def callback(status: JobStatus) -> None:
if self.dry_run or status != JobStatus.PASSED:
return

plugin = get_sim_tool_plugin(tool=self.sim_cfg.tool)
plugin = get_sim_tool_plugin(tool=self._typed_sim_cfg.tool)

results, self.cov_total = plugin.get_cov_summary_table(
cov_report_path=self.cov_report_txt,
Expand Down
Loading