From 92e98c1a2c46f207cdbf6aeb134c3c6d03c79fd9 Mon Sep 17 00:00:00 2001 From: Bryan Ngo <48371418+bdngo@users.noreply.github.com> Date: Mon, 13 Feb 2023 21:53:49 -0800 Subject: [PATCH] make dumping history opt-out by default (#709) * make dumping history opt-out by default * pass flag through history tests * Margins to_dict didn't str-ify Decimals * add flag to docs --------- Co-authored-by: Harrison Liew --- doc/Hammer-Use/Hammer-Config.rst | 5 ++-- hammer/vlsi/cli_driver.py | 44 ++++++++++++++++++++------------ hammer/vlsi/constraints.py | 8 +++--- hammer/vlsi/driver.py | 10 ++++++++ tests/test_cli_driver.py | 15 +++++++++-- 5 files changed, 58 insertions(+), 24 deletions(-) diff --git a/doc/Hammer-Use/Hammer-Config.rst b/doc/Hammer-Use/Hammer-Config.rst index 6a9a031d9..6a12e8e5f 100644 --- a/doc/Hammer-Use/Hammer-Config.rst +++ b/doc/Hammer-Use/Hammer-Config.rst @@ -238,6 +238,7 @@ Key History ----------- With the ``ruamel.yaml`` package, Hammer can emit what files have modified any configuration keys in YAML format. +Turning on key history is accomplished with the ``--dump-history`` command-line flag. The file is named ``{action}-output-history.yml`` and is located in the output folder of the given action. Example with the file ``test-config.yml``: @@ -254,7 +255,7 @@ Example with the file ``test-config.yml``: synthesis_tool: "hammer.synthesis.nop" -``test/syn-rundir/syn-output-history.yml`` after executing the command ``hammer-vlsi -p test-config.yml --obj_dir test syn``: +``test/syn-rundir/syn-output-history.yml`` after executing the command ``hammer-vlsi --dump-history -p test-config.yml --obj_dir test syn``: .. code-block:: yaml @@ -294,7 +295,7 @@ Example with the files ``test-config.yml`` and ``test-config2.yml``, respectivel foo.subst: "hammer.technology.nop2" -``test/syn-rundir/par-output-history.yml`` after executing the command ``hammer-vlsi -p test-config.yml -p test-config2.yml --obj_dir test syn-par``: +``test/syn-rundir/par-output-history.yml`` after executing the command ``hammer-vlsi --dump-history -p test-config.yml -p test-config2.yml --obj_dir test syn-par``: .. code-block:: yaml diff --git a/hammer/vlsi/cli_driver.py b/hammer/vlsi/cli_driver.py index 48268a5e4..cd91f7c9f 100644 --- a/hammer/vlsi/cli_driver.py +++ b/hammer/vlsi/cli_driver.py @@ -599,8 +599,9 @@ def action(driver: HammerDriver, append_error_func: Callable[[str], None]) -> Op dump_config_to_json_file(os.path.join(driver.syn_tool.run_dir, "syn-output.json"), output) dump_config_to_json_file(os.path.join(driver.syn_tool.run_dir, "syn-output-full.json"), self.get_full_config(driver, output)) - dump_config_to_yaml_file(os.path.join(driver.syn_tool.run_dir, "syn-output-history.yml"), - add_key_history(self.get_full_config(driver, output), key_history)) + if driver.dump_history: + dump_config_to_yaml_file(os.path.join(driver.syn_tool.run_dir, "syn-output-history.yml"), + add_key_history(self.get_full_config(driver, output), key_history)) post_run_func_checked(driver) elif action_type == "par": if not driver.load_par_tool(get_or_else(self.par_rundir, "")): @@ -618,8 +619,9 @@ def action(driver: HammerDriver, append_error_func: Callable[[str], None]) -> Op dump_config_to_json_file(os.path.join(driver.par_tool.run_dir, "par-output.json"), output) dump_config_to_json_file(os.path.join(driver.par_tool.run_dir, "par-output-full.json"), self.get_full_config(driver, output)) - dump_config_to_yaml_file(os.path.join(driver.par_tool.run_dir, "par-output-history.yml"), - add_key_history(self.get_full_config(driver, output), key_history)) + if driver.dump_history: + dump_config_to_yaml_file(os.path.join(driver.par_tool.run_dir, "par-output-history.yml"), + add_key_history(self.get_full_config(driver, output), key_history)) post_run_func_checked(driver) elif action_type == "drc": if not driver.load_drc_tool(get_or_else(self.drc_rundir, "")): @@ -637,8 +639,9 @@ def action(driver: HammerDriver, append_error_func: Callable[[str], None]) -> Op dump_config_to_json_file(os.path.join(driver.drc_tool.run_dir, "drc-output.json"), output) dump_config_to_json_file(os.path.join(driver.drc_tool.run_dir, "drc-output-full.json"), self.get_full_config(driver, output)) - dump_config_to_yaml_file(os.path.join(driver.drc_tool.run_dir, "drc-output-history.yml"), - add_key_history(self.get_full_config(driver, output), key_history)) + if driver.dump_history: + dump_config_to_yaml_file(os.path.join(driver.drc_tool.run_dir, "drc-output-history.yml"), + add_key_history(self.get_full_config(driver, output), key_history)) post_run_func_checked(driver) elif action_type == "lvs": if not driver.load_lvs_tool(get_or_else(self.lvs_rundir, "")): @@ -656,8 +659,9 @@ def action(driver: HammerDriver, append_error_func: Callable[[str], None]) -> Op dump_config_to_json_file(os.path.join(driver.lvs_tool.run_dir, "lvs-output.json"), output) dump_config_to_json_file(os.path.join(driver.lvs_tool.run_dir, "lvs-output-full.json"), self.get_full_config(driver, output)) - dump_config_to_yaml_file(os.path.join(driver.lvs_tool.run_dir, "lvs-output-history.yml"), - add_key_history(self.get_full_config(driver, output), key_history)) + if driver.dump_history: + dump_config_to_yaml_file(os.path.join(driver.lvs_tool.run_dir, "lvs-output-history.yml"), + add_key_history(self.get_full_config(driver, output), key_history)) post_run_func_checked(driver) elif action_type == "sram_generator": if not driver.load_sram_generator_tool(get_or_else(self.sram_generator_rundir, "")): @@ -689,8 +693,9 @@ def action(driver: HammerDriver, append_error_func: Callable[[str], None]) -> Op dump_config_to_json_file(os.path.join(driver.sim_tool.run_dir, "sim-output.json"), output) dump_config_to_json_file(os.path.join(driver.sim_tool.run_dir, "sim-output-full.json"), self.get_full_config(driver, output)) - dump_config_to_yaml_file(os.path.join(driver.sim_tool.run_dir, "sim-output-history.yml"), - add_key_history(self.get_full_config(driver, output), key_history)) + if driver.dump_history: + dump_config_to_yaml_file(os.path.join(driver.sim_tool.run_dir, "sim-output-history.yml"), + add_key_history(self.get_full_config(driver, output), key_history)) post_run_func_checked(driver) elif action_type == "power": if not driver.load_power_tool(get_or_else(self.power_rundir, "")): @@ -705,8 +710,9 @@ def action(driver: HammerDriver, append_error_func: Callable[[str], None]) -> Op dump_config_to_json_file(os.path.join(driver.power_tool.run_dir, "power-output.json"), output) dump_config_to_json_file(os.path.join(driver.power_tool.run_dir, "power-output-full.json"), self.get_full_config(driver, output)) - dump_config_to_yaml_file(os.path.join(driver.power_tool.run_dir, "power-output-history.yml"), - add_key_history(self.get_full_config(driver, output), key_history)) + if driver.dump_history: + dump_config_to_yaml_file(os.path.join(driver.power_tool.run_dir, "power-output-history.yml"), + add_key_history(self.get_full_config(driver, output), key_history)) post_run_func_checked(driver) elif action_type == "formal": if not driver.load_formal_tool(get_or_else(self.formal_rundir, "")): @@ -724,8 +730,9 @@ def action(driver: HammerDriver, append_error_func: Callable[[str], None]) -> Op dump_config_to_json_file(os.path.join(driver.formal_tool.run_dir, "formal-output.json"), output) dump_config_to_json_file(os.path.join(driver.formal_tool.run_dir, "formal-output-full.json"), self.get_full_config(driver, output)) - dump_config_to_yaml_file(os.path.join(driver.formal_tool.run_dir, "formal-output-history.yml"), - add_key_history(self.get_full_config(driver, output), key_history)) + if driver.dump_history: + dump_config_to_yaml_file(os.path.join(driver.formal_tool.run_dir, "formal-output-history.yml"), + add_key_history(self.get_full_config(driver, output), key_history)) post_run_func_checked(driver) elif action_type == "timing": if not driver.load_timing_tool(get_or_else(self.timing_rundir, "")): @@ -760,8 +767,9 @@ def action(driver: HammerDriver, append_error_func: Callable[[str], None]) -> Op dump_config_to_json_file(os.path.join(driver.pcb_tool.run_dir, "pcb-output.json"), output) dump_config_to_json_file(os.path.join(driver.pcb_tool.run_dir, "pcb-output-full.json"), self.get_full_config(driver, output)) - dump_config_to_yaml_file(os.path.join(driver.pcb_tool.run_dir, "pcb-output-history.yml"), - add_key_history(self.get_full_config(driver, output), key_history)) + if driver.dump_history: + dump_config_to_yaml_file(os.path.join(driver.pcb_tool.run_dir, "pcb-output-history.yml"), + add_key_history(self.get_full_config(driver, output), key_history)) post_run_func_checked(driver) else: raise ValueError("Invalid action_type = " + str(action_type)) @@ -1573,6 +1581,8 @@ def auto_action(driver: HammerDriver, append_error_func: Callable[[str], None]) self.hierarchical_auto_action = auto_action + driver.dump_history = args["dump_history"] + return driver, errors @staticmethod @@ -1695,6 +1705,8 @@ def main(self, args: Optional[List[str]] = None) -> None: help='Top module. If not specified, hammer-vlsi will take it from synthesis.inputs.top_module.') parser.add_argument("--cad-files", action='append', required=False, help="CAD files.") + parser.add_argument("--dump-history", default=False, action=argparse.BooleanOptionalAction, + help='Option to dump the key history of all the project configurations.') try: output = subprocess.check_output(["hammer-shell-test"]).decode().strip() diff --git a/hammer/vlsi/constraints.py b/hammer/vlsi/constraints.py index c9636a2ce..dfb5fb49e 100644 --- a/hammer/vlsi/constraints.py +++ b/hammer/vlsi/constraints.py @@ -575,10 +575,10 @@ def empty() -> "Margins": def to_dict(self) -> dict: return { - "left": self.left, - "bottom": self.bottom, - "right": self.right, - "top": self.top + "left": str(self.left), + "bottom": str(self.bottom), + "right": str(self.right), + "top": str(self.top) } diff --git a/hammer/vlsi/driver.py b/hammer/vlsi/driver.py index 4297af388..dda264ed9 100644 --- a/hammer/vlsi/driver.py +++ b/hammer/vlsi/driver.py @@ -132,6 +132,8 @@ def __init__(self, options: HammerDriverOptions, extra_project_config: dict = {} self.post_custom_timing_tool_hooks = [] # type: List[HammerToolHookAction] self.post_custom_pcb_tool_hooks = [] # type: List[HammerToolHookAction] + self._dump_history = False + @property def project_config(self) -> dict: return hammer_config.combine_configs(self.project_configs) @@ -1804,3 +1806,11 @@ def visit_module(mod: str) -> None: output.append((module, constraint_dict)) return (output, dependency_graph) + + @property + def dump_history(self) -> bool: + return self._dump_history + + @dump_history.setter + def dump_history(self, new_dump_history: bool) -> None: + self._dump_history = new_dump_history diff --git a/tests/test_cli_driver.py b/tests/test_cli_driver.py index 2f4e527da..c7327339e 100644 --- a/tests/test_cli_driver.py +++ b/tests/test_cli_driver.py @@ -457,7 +457,17 @@ def test_key_history(self, tmp_path) -> None: set_up_run(tmp_path) history_path = os.path.join(syn_rundir(tmp_path), "syn-output-history.yml") - run_syn_to_par_with_output(tmp_path) + with pytest.raises(SystemExit) as cm: + CLIDriver().main(args=[ + "syn", # action + "-p", config_path(tmp_path), + "--output", syn_out_path(tmp_path), + "--syn_rundir", syn_rundir(tmp_path), + "--obj_dir", obj_dir_path(tmp_path), + "--log", log_path(tmp_path), + "--dump-history" + ]) + assert cm.value.code == 0 # History file should have comments with open(config_path(tmp_path), 'r') as f: @@ -484,7 +494,8 @@ def test_key_history_as_input(self, tmp_path) -> None: "--output", syn_out_path(tmp_path), "--syn_rundir", syn_rundir(tmp_path), "--obj_dir", obj_dir_path(tmp_path), - "--log", log_path(tmp_path) + "--log", log_path(tmp_path), + "--dump-history" ]) assert cm.value.code == 0