Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 14 additions & 10 deletions .agents/contributing-changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,12 @@ scope rules that decide how a feature is received:

## Formatting: pre-commit is required

`.pre-commit-config.yaml` runs ruff (with `--fix`) and ruff-format, flake8 with a 125-character line limit
plus bugbear and print checks, pyupgrade targeting Python 3.10, pyproject-fmt, the standard whitespace and
YAML hooks, and `p-clang-format` for C and C++ — so edited `nnet_utils` headers are reformatted too.
`.pre-commit-config.yaml` runs ruff (with `--fix`) and ruff-format, pyupgrade targeting Python 3.10,
pyproject-fmt, the standard whitespace and YAML hooks, and `p-clang-format` for C and C++ — so edited
`nnet_utils` headers are reformatted too. Ruff's rules live in `[tool.ruff]` in `pyproject.toml`: a
125-character line limit plus pyflakes, isort, and most pycodestyle and bugbear checks. They include
`PLW2901`, which rejects reassigning a loop variable inside its loop, as in `line = line.replace(...)`;
give the loop variable or the result a name of its own.

```
pre-commit run --files <the files you changed>
Expand All @@ -53,11 +56,12 @@ pre-commit run --files <the files you changed>
The first run of a hook installs its own environment and needs network access. Commit the reformatting the
hooks apply; pull requests that fail these checks are not reviewed.

**That command does not run everything CI runs.** Hooks marked `stages: [manual]` in
`.pre-commit-config.yaml` are skipped by default, and the Format workflow runs pre-commit with
`--hook-stage manual --all-files`. `check-manifest` is one of those hooks: it builds an sdist and compares it
against the files git tracks, so **adding, moving or renaming anything outside the packaged directories can
fail CI while every local check passes**. If your change touches the repository layout, run what CI runs:
**That command does not run everything CI runs.** CI splits the hooks between two services: pre-commit.ci
runs the default-stage hooks on every pull request, and the `Check manifest` workflow runs the one hook
pre-commit.ci cannot, `check-manifest`. That hook is marked `stages: [manual]` in `.pre-commit-config.yaml`,
so it is skipped by a plain local run. It builds an sdist and compares it against the files git tracks, so
**adding, moving or renaming anything outside the packaged directories can fail CI while every local check
passes**. If your change touches the repository layout, run what CI runs:

```
pre-commit run --hook-stage manual --all-files
Expand Down Expand Up @@ -161,8 +165,8 @@ fails on both is the environment's.
Run this over your own diff before anyone else sees it. Most review comments on generated contributions come
from this list, and the linters catch almost none of it.

- **Debug output.** `flake8-print` is configured but `T201` is in `extend-ignore`, so stray `print()` calls
pass CI. hls4ml prints deliberately in some passes to warn users; your debugging does not belong there.
- **Debug output.** No hook checks for `print()`, so stray calls pass CI. hls4ml prints deliberately in some
passes to warn users; your debugging does not belong there.
- **Commented-out code and dead branches.** Delete them. A reviewer cannot tell your leftovers from
deliberate ones.
- **Dead copied code.** When a backend, pass or kernel was copied as a starting point, remove everything the
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: Run pre-commit
name: Check manifest

on:
pull_request:
Expand All @@ -7,20 +7,18 @@ on:
branches: [ main ]

jobs:
pre-commit:
name: Format
check-manifest:
runs-on: ubuntu-latest
strategy:
matrix:
python-version: [3.8]

steps:
- name: Checkout
uses: actions/checkout@v7
with:
submodules: recursive

# pre-commit.ci runs the default-stage hooks on every pull request, but not
# check-manifest, which is manual-stage because it builds an sdist.
- name: Pre-commit
uses: pre-commit/action@v3.0.1
with:
extra_args: --hook-stage manual --all-files
extra_args: --hook-stage manual --all-files check-manifest
14 changes: 0 additions & 14 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,6 @@ repos:
- id: pyupgrade
args: ["--py310-plus"]

- repo: https://github.com/pycqa/flake8

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Removing flake8 also removes pycodestyle E1xx/E2xx/E3xx and W391. In ruff those rules are preview-only, and E/W in lint.select enable only the stable ones. ruff-format fixes most of these, but not inside # fmt: off blocks or in comment spacing. The check that today's tree passes shows nothing breaks now, not that future code would still be caught. Is losing these acceptable, or should preview = true (or an explicit preview selection) be considered?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

They're back. lint.preview = true and lint.explicit-preview-rules = true are now set, and
lint.extend-select lists the 38 preview pycodestyle codes flake8 ran. flake8 skipped E203, E226, E241 and
E242 through its default and --extend-ignore lists, so those stay off. On the tree they report nothing.

Some details:

  • Prefixes still select stable rules only. With explicit-preview-rules, selections like E and B
    don't pick up preview rules.
  • Preview changes stable rules too. lint.preview also turns on preview behaviour for the stable rules.
    That adds no findings today, but it can change with a ruff release. If it does, it will show up on
    pre-commit.ci's quarterly autoupdate PR, not on unrelated PRs.
  • Rule codes in output. Preview makes ruff print rule names instead of codes. output-prefer-rule-codes = true undoes that. # noqa: <code> works either way.
  • Still not checked: E12x. pycodestyle's E12x continuation-line checks have no ruff implementation, stable
    or preview. ruff-format covers them everywhere except inside # fmt: off.

rev: 7.4.1
hooks:
- id: flake8
exclude: docs/conf.py
additional_dependencies: [flake8-bugbear, flake8-print]
args: ['--max-line-length=125', # github viewer width
'--extend-ignore=E203,T201,F401',
# E203 is not PEP8 compliant
# F401 included in ruff (behaves slightly differently for noqa flags)
'--per-file-ignores=hls4ml/model/optimizer/passes/bit_exact.py:E741',
# i for #int w/o sign, I for #int w/ sign when massively processing bw conversions
]

- repo: https://github.com/mgedmin/check-manifest
rev: "0.51"
hooks:
Expand Down
4 changes: 2 additions & 2 deletions hls4ml/backends/altera/altera_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,8 @@ def build(self, model, build_type='fpga_emu', run=False):
builddir.mkdir(exist_ok=True)
try:
subprocess.run('which ahls', shell=True, cwd=builddir, check=True)
except subprocess.CalledProcessError:
raise RuntimeError('Could not find ahls. Please configure the Altera HLS IP Gen toolchain appropriately')
except subprocess.CalledProcessError as e:
raise RuntimeError('Could not find ahls. Please configure the Altera HLS IP Gen toolchain appropriately') from e
subprocess.run('cmake ..', shell=True, cwd=builddir, check=True)
subprocess.run(f'make {build_type}', shell=True, cwd=builddir, check=True)

Expand Down
4 changes: 2 additions & 2 deletions hls4ml/backends/vivado/passes/distributed_arithmetic.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def _(layer: Conv1D | Conv2D):
def _(node: EinsumDense):
inp_tpose_idx = node.attributes['inp_tpose_idxs']
L = node.attributes['n_free_data']
I = node.attributes['n_inplace'] # noqa: E741
I = node.attributes['n_inplace']
C = node.attributes['n_contract']

k, i, f = _get_input_kif(node)
Expand Down Expand Up @@ -173,7 +173,7 @@ def match(self, node: Layer):
def transform(self, model: 'ModelGraph', node: FixedPointQuantizer):
for out_layer in get_output_layers(node):
k, i, f = get_kernel_inp_kif(out_layer)
B, I = i + f + k, i + k # noqa: E741
B, I = i + f + k, i + k

quantization_lines, replaces = [], []
for i, (_k, _B, _I) in enumerate(zip(k, B, I)):
Expand Down
4 changes: 2 additions & 2 deletions hls4ml/converters/keras/hgq_proxy_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ def unary_lut_keras_handler(keras_layer, input_names, input_shapes, data_reader:

table = data_reader.get_weights_data(config['name'], 'table')
k, i, f = keras_layer['config']['kif_out']
k, b, I = k, k + i + f, k + i # noqa: E741
config['table_t'] = FixedPrecisionType(b, I, k) # noqa: E741
k, b, I = k, k + i + f, k + i
config['table_t'] = FixedPrecisionType(b, I, k)
config['table_data'] = table
config['activation'] = 'unary_lut'

Expand Down
10 changes: 5 additions & 5 deletions hls4ml/converters/keras_v3/hgq2/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,18 @@ def extract_fixed_quantizer_config(q, tensor: 'KerasTensor', is_input: bool) ->
if any([s is None for s in shape]):
raise ValueError(f'Tensor {tensor.name} has at least one dimension with no fixed size')
k, i, f = internal_q.kif
k, B, I = k, k + i + f, k + i # type: ignore # noqa: E741
k, B, I = ops.convert_to_numpy(k), ops.convert_to_numpy(B), ops.convert_to_numpy(I) # noqa: E741
I = np.where(B > 0, I, 0) # noqa: E741 # type: ignore
k, B, I = k, k + i + f, k + i # type: ignore
k, B, I = ops.convert_to_numpy(k), ops.convert_to_numpy(B), ops.convert_to_numpy(I)
I = np.where(B > 0, I, 0) # type: ignore

if np.size(k) != 1:
k = np.broadcast_to(k.astype(np.int16), (1,) + shape) # type: ignore
B = np.broadcast_to(B.astype(np.int16), (1,) + shape) # type: ignore
I = np.broadcast_to(I.astype(np.int16), (1,) + shape) # noqa: E741
I = np.broadcast_to(I.astype(np.int16), (1,) + shape)
else:
k = np.ravel(k).astype(np.int16)
B = np.ravel(B).astype(np.int16)
I = np.ravel(I).astype(np.int16) # noqa: E741
I = np.ravel(I).astype(np.int16)

overflow_mode: str = internal_q.overflow_mode
round_mode: str = internal_q.round_mode
Expand Down
2 changes: 1 addition & 1 deletion hls4ml/converters/keras_v3/hgq2/softmax.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def fixed_quantizer_to_hls4ml_t(q: 'FixedPointQuantizerBase', take_max=False):
i = int(i.max())
f = int(f.max())

k, b, I = k, k + i + f, k + i # noqa: E741
k, b, I = k, k + i + f, k + i
b = max(1, b)
round_mode = q.round_mode
if round_mode.startswith('S_'):
Expand Down
2 changes: 1 addition & 1 deletion hls4ml/converters/keras_v3/hgq2/unary_lut.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def handle(
k, i, f = (ops.convert_to_numpy(x).ravel().item() for x in oq.kif)
table = fixed_q(table, k, i, f) # type: ignore

k, b, I = bool(k), k + i + f, k + i # noqa: E741
k, b, I = bool(k), k + i + f, k + i
table_t = FixedPrecisionType(b, I, k)
else:
assert isinstance(oq, FloatPointQuantizer)
Expand Down
8 changes: 4 additions & 4 deletions hls4ml/converters/keras_v3/pquant/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ def extract_pquant_quantizer_config(q, tensor: 'KerasTensor', is_input: bool = N
raise ValueError(f'Tensor {tensor.name} has at least one dimension with no fixed size')

k, i, f = q.quantizer.quantizer.kif if q.use_hgq else (q.k, q.i, q.f)
k, B, I = k, k + i + f, k + i # type: ignore # noqa: E741
k, B, I = ops.convert_to_numpy(k), ops.convert_to_numpy(B), ops.convert_to_numpy(I) # noqa: E741
I = np.where(B > 0, I, 0) # noqa: E741 # type: ignore
k, B, I = k, k + i + f, k + i # type: ignore
k, B, I = ops.convert_to_numpy(k), ops.convert_to_numpy(B), ops.convert_to_numpy(I)
I = np.where(B > 0, I, 0) # type: ignore

k = np.broadcast_to(k.astype(np.int16), (1,) + shape) # type: ignore
B = np.broadcast_to(B.astype(np.int16), (1,) + shape) # type: ignore
I = np.broadcast_to(I.astype(np.int16), (1,) + shape) # noqa: E741
I = np.broadcast_to(I.astype(np.int16), (1,) + shape)

overflow_mode: str = getattr(q, 'overflow_mode' if q.use_hgq else 'overflow', 'SAT')
round_mode: str = q.round_mode
Expand Down
3 changes: 2 additions & 1 deletion hls4ml/converters/pytorch/convolution.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ def parse_conv_transpose_layer(operation, layer_name, input_names, input_shapes,
# the end, or as many as the `output_size` of the call needs
axes = [('width', 'left')] if len(px_in_shape) == 1 else [('height', 'top'), ('width', 'left')]
px_out_shape = []
for (axis, side), n_in, k, s, d, p, op, n_out in zip(
for (axis, side), n_in, k, s, d, p, op, out_size in zip(
axes,
px_in_shape,
class_object.kernel_size,
Expand All @@ -178,6 +178,7 @@ def parse_conv_transpose_layer(operation, layer_name, input_names, input_shapes,
class_object.output_padding,
output_size or [None] * len(px_in_shape),
):
n_out = out_size
if n_out is None:
n_out = (n_in - 1) * s - 2 * p + d * (k - 1) + op + 1
layer[f'in_{axis}'] = n_in
Expand Down
8 changes: 4 additions & 4 deletions hls4ml/converters/pytorch/pquant.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ def parse_quantizer_layer(operation, layer_name, input_names, input_shapes, node
if any([s is None for s in shape]):
raise ValueError(f'Tensor {input_names[0]} has at least one dimension with no fixed size')
k, i, f = q_params['k'].data, q_params['i'].data, q_params['f'].data
k, B, I = k, k + i + f, k + i # type: ignore # noqa: E741
k, B, I = k.detach().cpu().numpy(), B.detach().cpu().numpy(), I.detach().cpu().numpy() # noqa: E741
I = np.where(B > 0, I, 0) # noqa: E741 # type: ignore
k, B, I = k, k + i + f, k + i # type: ignore
k, B, I = k.detach().cpu().numpy(), B.detach().cpu().numpy(), I.detach().cpu().numpy()
I = np.where(B > 0, I, 0) # type: ignore

k = np.broadcast_to(k.astype(np.int16), (1,) + shape) # type: ignore
B = np.broadcast_to(B.astype(np.int16), (1,) + shape) # type: ignore
I = np.broadcast_to(I.astype(np.int16), (1,) + shape) # noqa: E741
I = np.broadcast_to(I.astype(np.int16), (1,) + shape)

overflow_mode: str = class_object.overflow
round_mode: str = class_object.round_mode
Expand Down
14 changes: 8 additions & 6 deletions hls4ml/converters/pytorch_to_hls.py
Original file line number Diff line number Diff line change
Expand Up @@ -279,15 +279,17 @@ def resolve_getitem_source(node_name, visited=None):
for arg in node.args:
if isinstance(arg, tuple):
for input in arg:
source = input
if hasattr(input, 'name') and 'getitem' in input.name:
input = resolve_getitem_source(input.name)
input_shapes.append(output_shapes[str(input)])
input_names.append(inputs_map.get(str(input), str(input)))
source = resolve_getitem_source(input.name)
input_shapes.append(output_shapes[str(source)])
input_names.append(inputs_map.get(str(source), str(source)))
else:
source = arg
if hasattr(arg, 'name') and 'getitem' in arg.name:
arg = resolve_getitem_source(arg.name)
input_shapes.append(output_shapes[str(arg)])
input_names.append(inputs_map.get(str(arg), str(arg)))
source = resolve_getitem_source(arg.name)
input_shapes.append(output_shapes[str(source)])
input_names.append(inputs_map.get(str(source), str(source)))

# if a 'getitem' is the input to a node, step back in the graph to find the real source of the input
elif 'getitem' in node.args[0].name:
Expand Down
8 changes: 5 additions & 3 deletions hls4ml/model/layers.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,9 @@ def __init__(self, model, name, attributes, inputs, outputs=None, initialize=Tru
self._set_accum_t()

layer_config = self.model.config.get_layer_config(self)
for config_key, config_value in layer_config.items():
config_key = convert_to_snake_case(config_key)
for key, value in layer_config.items():
config_key = convert_to_snake_case(key)
config_value = value
if config_key.endswith('_t') and isinstance(
config_value, str
): # TODO maybe move this to __setitem__ of AttributeDict?
Expand Down Expand Up @@ -156,7 +157,8 @@ def _validate_attributes(self):
all_attributes[attr.name] = attr

# Validate existing attributes
for attr_name, attr_value in self.attributes.items():
for attr_name, value in self.attributes.items():
attr_value = value
if isinstance(attr_value, PrecisionType):
attr_value = self._wrap_precision_to_type(f'{self.name}_{attr_name}', attr_value)
self.set_attr(attr_name, attr_value)
Expand Down
5 changes: 3 additions & 2 deletions hls4ml/model/optimizer/passes/bit_exact.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,12 @@ def get_input_layers(layer: Layer) -> list[Layer]:
inp_names = layer.inputs
ret = []
for name in inp_names:
graph_name = name
if name not in model.graph.keys(): # in stream_io, <name>_cpt\d+ may be used instead of <name>
matched = rm_cpy.match(name)
assert matched, f'Layer {layer.name} has input {name} which is not in the model (keys: {model.graph.keys()})'
name = matched.group('name')
ret.append(model.graph[name])
graph_name = matched.group('name')
ret.append(model.graph[graph_name])
return ret


Expand Down
8 changes: 4 additions & 4 deletions hls4ml/model/profiling.py
Original file line number Diff line number Diff line change
Expand Up @@ -385,9 +385,9 @@ def weights_keras(model, fmt='longform', plot='boxplot'):
name = layer.name
weights, suffix = keras_process_layer_map[type(layer).__name__](layer)

for i, w in enumerate(weights):
for i, weight in enumerate(weights):
label = f'{name}/{suffix[i]}'
w = w.flatten()
w = weight.flatten()
w = abs(w[w != 0])
n = len(w)
if n == 0:
Expand Down Expand Up @@ -421,9 +421,9 @@ def activations_keras(model, X, fmt='longform', plot='boxplot'):
[layer for layer in model.layers if not isinstance(layer, keras.layers.InputLayer)], X, model.input
)
outputs = dict(zip([layer.name for layer in model.layers if not isinstance(layer, keras.layers.InputLayer)], outputs))
for layer_name, y in outputs.items():
for layer_name, output in outputs.items():
print(f' {layer_name}')
y = y.flatten()
y = output.flatten()
y = abs(y[y != 0])
if len(y) == 0:
print(f'Activations for {layer_name} are only zeros, ignoring.')
Expand Down
4 changes: 2 additions & 2 deletions hls4ml/optimization/dsp_aware_pruning/keras/reduction.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ def reduce_model(model):
"""
try:
from kerassurgeon import Surgeon
except ModuleNotFoundError:
except ModuleNotFoundError as e:
raise Exception(
'Keras Surgeon not installed. Unable to reduce model footprint '
'Please install up-to-date Keras Surgeon compatible with TensorFlow 2.3+ and QKeras '
'Installation from git: https://github.com/fastmachinelearning/keras-surgeon'
)
) from e

# Initiate surgeon
surgeon = Surgeon(model)
Expand Down
8 changes: 4 additions & 4 deletions hls4ml/optimization/dsp_aware_pruning/knapsack.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,8 @@ def __solve_knapsack_branch_and_bound(values, weights, capacity, time_limit=sys.
"""
try:
from ortools.algorithms import pywrapknapsack_solver
except ModuleNotFoundError:
raise Exception('OR-Tools not found. Please insteal Google OR-Tools from pip.')
except ModuleNotFoundError as e:
raise Exception('OR-Tools not found. Please insteal Google OR-Tools from pip.') from e

solver = pywrapknapsack_solver.KnapsackSolver(
pywrapknapsack_solver.KnapsackSolver.KNAPSACK_MULTIDIMENSION_BRANCH_AND_BOUND_SOLVER, 'BB'
Expand All @@ -222,8 +222,8 @@ def __solve_knapsack_cbc_mip(values, weights, capacity, time_limit=sys.float_inf
"""
try:
from ortools.algorithms import pywrapknapsack_solver
except ModuleNotFoundError:
raise Exception('OR-Tools not found. Please insteal Google OR-Tools from pip.')
except ModuleNotFoundError as e:
raise Exception('OR-Tools not found. Please insteal Google OR-Tools from pip.') from e

solver = pywrapknapsack_solver.KnapsackSolver(
pywrapknapsack_solver.KnapsackSolver.KNAPSACK_MULTIDIMENSION_CBC_MIP_SOLVER, 'CBC'
Expand Down
4 changes: 2 additions & 2 deletions hls4ml/report/vivado_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -627,8 +627,8 @@ def _make_vivado_tables(vivado_report, make_table_template, titles=('Resource ut
}

tables = make_table_template(titles[0], area_rows)
for title, rows in [(titles[1], timing_rows), (titles[2], power_rows)]:
rows = {row_title: key for row_title, key in rows.items() if key in vivado_report}
for title, candidate_rows in [(titles[1], timing_rows), (titles[2], power_rows)]:
rows = {row_title: key for row_title, key in candidate_rows.items() if key in vivado_report}
if rows:
tables += make_table_template(title, rows)

Expand Down
12 changes: 6 additions & 6 deletions hls4ml/utils/einsum_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,20 @@ def _validate_einsum_expr(fn: str, shape0: tuple[int, ...], shape1: tuple[int, .
for a in in0:
if in0.count(a) == 1:
continue
a = a if a != '0' else '...'
raise ValueError(f"einsum string {fn} is invalid: input0 subscripts includes '{a}' multiple times")
subscript = a if a != '0' else '...'
raise ValueError(f"einsum string {fn} is invalid: input0 subscripts includes '{subscript}' multiple times")
if len(sax_in1) != len(ax_in1):
for a in in1:
if in1.count(a) == 1:
continue
a = a if a != '0' else '...'
raise ValueError(f"einsum string {fn} is invalid: input1 subscripts includes '{a}' multiple times")
subscript = a if a != '0' else '...'
raise ValueError(f"einsum string {fn} is invalid: input1 subscripts includes '{subscript}' multiple times")
if len(sax_out) != len(ax_out):
for a in out:
if out.count(a) == 1:
continue
a = a if a != '0' else '...'
raise ValueError(f"einsum string {fn} is invalid: output subscripts includes '{a}' multiple times")
subscript = a if a != '0' else '...'
raise ValueError(f"einsum string {fn} is invalid: output subscripts includes '{subscript}' multiple times")

# Invalid broadcasting
if '0' in sax_in0 or '0' in sax_in1 or '0' in sax_out:
Expand Down
Loading
Loading