Skip to content

[Task] Added Ability to Constrain Circuits in Config #2780

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

Conversation

AlexandreSinger
Copy link
Contributor

The VTR task interface uses configuration files to specify what circuits to run for a given task. The current implementation of these config files only allowed the user to run all of their circuits on the same device and using the same flat constraint file (for example fixing the IOs). This was only able to be done through specifying them in the script_params option, but this was limited.

This causes problems for the AP flow since each circuit has different IO constraints and may be constrained to different devices or architectures. The only way to use the VTR tasks is to create one configuration file per circuit, which is very wasteful and challenging to work with. It also makes parsing the QoR more difficult.

Added a new configuration option that can be added to the configuration file which can constrain a circuit to a given option. The syntax is as follows:
circuit_constraint_list_add=(, <constr_key, constr_val>)
This line will tell VTR run task to constrain the circuit on the constr_key option.

The currently supported constraint keys are:
- arch: Constrain the circuit to only run on the given arch
- device: Constrain the circuit to only use the given device
- constraints: Constrain the circuit's atom placement

If a constraint is not specified, the bahaviour is unchanged. So, for example, if an arch constraint is not specified, the circuit will run on all the architectures in the arch_list (as usual).

Future work is to support constraining the route_chan_width so different circuits can be run with different channel widths in the same config file. This can easily be added using this interface.

Example of the interface in use:
Screenshot from 2024-10-17 16-12-47

@github-actions github-actions bot added the lang-python Python code label Oct 17, 2024
@AlexandreSinger
Copy link
Contributor Author

AlexandreSinger commented Oct 17, 2024

@vaughnbetz This is passing CI. The only test that it is failing is Python Lint due to me having to add 5 lines to the create_job method (which was already quite large); and now the linter is complaining that the function is 2 lines too long now:

def create_job(
args,
config,
circuit,
include,
arch,
noc_flow,
param,
cmd,
parse_cmd,
second_parse_cmd,
qor_parse_command,
work_dir,
run_dir,
golden_results,
) -> Job:
"""
Create an individual job with the specified parameters
"""
param_string = "common" + (("_" + param.replace(" ", "_")) if param else "")
# remove any address-related characters that might be in the param_string
# To avoid creating invalid URL path
path_str = "../"
if path_str in param_string:
param_string = param_string.replace("../", "")
param_string = param_string.replace("-", "")
circuit_2 = circuit.replace(".blif", "")
if circuit_2 in param_string:
ind = param_string.find(circuit_2)
param_string = param_string[ind + len(circuit_2) + 1 :]
for spec_char in [":", "<", ">", "|", "*", "?", "/", "."]:
# replaced to create valid URL path
param_string = param_string.replace(spec_char, "_")
if not param:
param = "common"
expected_min_w = ret_expected_min_w(circuit, arch, golden_results, param)
expected_min_w = (
int(expected_min_w * args.minw_hint_factor)
if hasattr(args, "minw_hint_factor")
else expected_min_w
)
expected_min_w += expected_min_w % 2
if expected_min_w > 0:
cmd += ["--min_route_chan_width_hint", str(expected_min_w)]
expected_vpr_status = ret_expected_vpr_status(arch, circuit, golden_results, param)
if expected_vpr_status not in ("success", "Unknown"):
cmd += ["-expect_fail", expected_vpr_status]
current_parse_cmd = parse_cmd.copy()
if config.parse_file:
current_parse_cmd += [
"arch={}".format(arch),
"circuit={}".format(circuit),
"noc_flow={}".format(noc_flow),
"script_params={}".format(load_script_param(param)),
]
current_parse_cmd.insert(0, run_dir + "/{}".format(load_script_param(param)))
current_second_parse_cmd = second_parse_cmd.copy() if second_parse_cmd else None
if config.second_parse_file:
current_second_parse_cmd += [
"arch={}".format(arch),
"circuit={}".format(circuit),
"noc_flow={}".format(noc_flow),
"script_params={}".format(load_script_param(param)),
]
current_second_parse_cmd.insert(0, run_dir + "/{}".format(load_script_param(param)))
current_qor_parse_command = qor_parse_command.copy() if qor_parse_command else None
if config.qor_parse_file:
current_qor_parse_command += [
"arch={}".format(arch),
"circuit={}".format(circuit),
"noc_flow={}".format(noc_flow),
"script_params={}".format("common"),
]
current_qor_parse_command.insert(0, run_dir + "/{}".format(load_script_param(param)))
current_cmd = cmd.copy()
current_cmd += ["-temp_dir", run_dir + "/{}".format(param_string)]
if getattr(args, "use_previous", None):
for prev_run, [extension, option] in args.use_previous:
prev_run_dir = get_existing_run_dir(find_task_dir(config, args.alt_tasks_dir), prev_run)
prev_work_path = Path(prev_run_dir) / work_dir / param_string
prev_file = prev_work_path / "{}.{}".format(Path(circuit).stem, extension)
if option == "REPLACE_BLIF":
current_cmd[0] = str(prev_file)
current_cmd += ["-start", "vpr"]
else:
current_cmd += [option, str(prev_file)]
if param_string != "common":
current_cmd += param.split(" ")
return Job(
config.task_name,
arch,
circuit,
include,
param_string,
work_dir + "/" + param_string,
current_cmd,
current_parse_cmd,
current_second_parse_cmd,
current_qor_parse_command,
)

I am thinking of allowing this function to be a bit long by disabling this check for this function. What do you think? If not it may be a bit of work to clean up the method.

image

@vaughnbetz
Copy link
Contributor

I'd rather either factor out something from the function, or, if you think that is a pain, just change the linting parameters to allow a few more statements per function. I'd rather not pragma off linting for a specific function, which could then grow without bound.

The VTR task interface uses configuration files to specify what circuits
to run for a given task. The current implementation of these config
files only allowed the user to run all of their circuits on the same
device and using the same flat constraint file (for example fixing the
IOs). This was only able to be done through specifying them in the
script_params option, but this was limited.

This causes problems for the AP flow since each circuit has different IO
constraints and may be constrained to different devices or
architectures. The only way to use the VTR tasks is to create one
configuration file per circuit, which is very wasteful and challenging
to work with. It also makes parsing the QoR more difficult.

Added a new configuration option that can be added to the configuration
file which can constrain a circuit to a given option. The syntax is as
follows:
  circuit_constraint_list_add=(<circuit>, <constr_key, constr_val>)
This line will tell VTR run task to constrain the circuit on the
constr_key option.

The currently supported constraint keys are:
        - arch: Constrain the circuit to only run on the given arch
        - device: Constrain the circuit to only use the given device
        - constraints: Constrain the circuit's atom placement

If a constraint is not specified, the bahaviour is unchanged. So, for
example, if an arch constraint is not specified, the circuit will run on
all the architectures in the arch_list (as usual).

Future work is to support constraining the route_chan_width so different
circuits can be run with different channel widths in the same config
file. This can easily be added using this interface.
@AlexandreSinger AlexandreSinger force-pushed the feature-vtr-task-ap-upgrade branch from f698668 to 4b45eda Compare October 18, 2024 00:26
@AlexandreSinger
Copy link
Contributor Author

@vaughnbetz I took your advice and outlined my contributions to the function into its own method. If we add more constraints in the future, this will make it easier so I think that was a good idea. The linter is now passing and the code is ready for review.

Copy link
Contributor

@vaughnbetz vaughnbetz left a comment

Choose a reason for hiding this comment

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

Looks good to me! I second the to-do to add a path to the constraints to clean up the syntax some.

@vaughnbetz vaughnbetz merged commit 24b3bd3 into verilog-to-routing:master Oct 18, 2024
37 of 53 checks passed
@AlexandreSinger AlexandreSinger deleted the feature-vtr-task-ap-upgrade branch November 27, 2024 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-python Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants