Skip to content
Closed
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
27 changes: 15 additions & 12 deletions .agents/contributing-changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,22 +42,25 @@ 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 the pycodestyle, pyflakes, isort and bugbear checks.

```
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.
The first run of a hook installs its own environment and needs network access. Hook revisions are pinned, so
a local run resolves to the same tools CI uses. 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 +164,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
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/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
6 changes: 3 additions & 3 deletions hls4ml/utils/plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def model_to_dot(
layers = model.get_layers()

# Create graph nodes.
for i, layer in enumerate(layers):
for layer in layers:
# layer_id = str(id(layer))
layer_id = str(layer.index)

Expand Down Expand Up @@ -112,8 +112,8 @@ def format_shape(shape):
if class_name != 'Input':
if len(layer.inputs) > 1:
input_shapes = []
for i in layer.inputs:
input_layer = layer.get_input_variable(i)
for input_name in layer.inputs:
input_layer = layer.get_input_variable(input_name)
if input_layer is not None:
input_shapes.append(input_layer.shape)
else:
Expand Down
4 changes: 2 additions & 2 deletions hls4ml/writer/catapult_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,8 @@ def write_project_cpp(self, model):
newline += ' ' + func[0] + ' // ' + layer.name + '\n'
else:
newline += ' // ' + layer.name + '\n'
for line in func:
newline += ' ' + line + '\n'
for func_line in func:
newline += ' ' + func_line + '\n'
if model.config.trace_output and layer.get_attr('trace', False):
newline += '#ifndef __SYNTHESIS__\n'
for var in vars:
Expand Down
4 changes: 2 additions & 2 deletions hls4ml/writer/coyote_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ def write_project_cpp(self, model):
newline += ' ' + func[0] + ' // ' + layer.name + '\n'
else:
newline += ' // ' + layer.name + '\n'
for line in func:
newline += ' ' + line + '\n'
for func_line in func:
newline += ' ' + func_line + '\n'
if model.config.trace_output and layer.get_attr('trace', False):
vars = layer.get_variables()
newline += '#ifndef __SYNTHESIS__\n'
Expand Down
4 changes: 2 additions & 2 deletions hls4ml/writer/libero_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,8 @@ def write_project_cpp(self, model):
newline += ' ' + func[0] + ' // ' + layer.name + '\n'
else:
newline += ' // ' + layer.name + '\n'
for line in func:
newline += ' ' + line + '\n'
for func_line in func:
newline += ' ' + func_line + '\n'
if model.config.trace_output and layer.get_attr('trace', False):
newline += '#ifndef __SYNTHESIS__\n'
for var in vars:
Expand Down
4 changes: 2 additions & 2 deletions hls4ml/writer/vivado_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,8 @@ def write_project_cpp(self, model):
newline += ' ' + func[0] + ' // ' + layer.name + '\n'
else:
newline += ' // ' + layer.name + '\n'
for line in func:
newline += ' ' + line + '\n'
for func_line in func:
newline += ' ' + func_line + '\n'
if model.config.trace_output and layer.get_attr('trace', False):
vars = layer.get_variables()
newline += '#ifndef __SYNTHESIS__\n'
Expand Down
8 changes: 5 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,16 @@ write_to = "hls4ml/_version.py"
target-version = "py310"
line-length = 125
indent-width = 4
include = [ "hls4ml/**/*.py", "tests/**/*.py" ]
include = [ "hls4ml/**/*.py", "test/**/*.py" ]
exclude = [ "hls4ml/_version.py", "hls4ml/templates/**" ]
format.quote-style = "single"
format.skip-magic-trailing-comma = false
format.docstring-code-line-length = 125
format.docstring-code-format = true
lint.select = [ "E", "F", "F401", "I", "W" ]
lint.ignore = [ "E741" ]
# PLE0100, PLE0101, PLW0129 and RUF024 stand in for flake8-bugbear B037, B044 and B046, which ruff has no B code for
lint.select = [ "B", "E", "F", "F401", "I", "PLE0100", "PLE0101", "PLW0129", "RUF024", "W" ]
# B028 and B905 need a per-call judgement (which stacklevel; strict=True or False), not a mechanical fix
lint.ignore = [ "B028", "B905", "E741" ]
lint.per-file-ignores = { "__init__.py" = [ "F401" ] }
lint.fixable = [ "ALL" ]
lint.unfixable = []
Expand Down
Loading