From ccf8a55a1fd86d6cfb584d432ab6322fa3e9bc2d Mon Sep 17 00:00:00 2001 From: Jan-Frederik Schulte Date: Mon, 3 Aug 2026 13:49:51 -0400 Subject: [PATCH 1/4] add AI tool usage section to CONTRIBUTING.md --- CONTRIBUTING.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3bd7b1e4f2..678eacd11d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -26,6 +26,18 @@ Consult [GitHub Help](https://help.github.com/articles/about-pull-requests/) for We will review your contribution and, if any additional fixes or modifications are necessary, may provide feedback to guide you. When accepted, your pull request will be merged to the repository. +## Use of AI tools + +Usage of AI tools in development for hls4ml is generally allowed. However, we require all contributers to adhere to the following guidelines: + +- Contributed code must still be you own original work. It is your responsibility to make sure that the generated code is compatible with the [hls4ml license](LICENSE.txt), [these Contributor Guidelines](CONTRIBUTING.md), and it doesn't violate the lincense of either the AI tool or any third-party license obligations. +- The AI tool name and version must be disclosed in the PR description. +- Ensure you have reviewed and fully understand the generated code and be prepared to explain the reasoning behind it during review. +- AI coding agents tend to be very verbose. Please review generated code and especially comments and trim unnecessary clutter. +- Do not spam the repository with issues and pull request for problems that have no likelihood of appearing in real world applications. Focus instead on fixing or improving hls4ml for real users. + +Submissions that appear unreviewed or copied directly from an AI tool without proper understanding may be requested to be revised or declined. + ## Code of Conduct In the interest of fostering an open and welcoming environment, we as From 5ca98ce90302013232a440a153cf883df80c26f1 Mon Sep 17 00:00:00 2001 From: vloncar Date: Fri, 14 Aug 2026 01:08:00 +0200 Subject: [PATCH 2/4] Extend AI policy, rework PR and issue templates, add AGENTS.md Builds on the AI tool usage section with the points raised in review: - CONTRIBUTING.md: fix typos and the LICENSE link; state that PRs are squash-merged and that the PR description is the permanent record; require that quoted numbers come from real runs; forbid crediting AI tools as commit authors, since authorship carries copyright a tool cannot hold and the trailers distort contributor statistics. - PULL_REQUEST_TEMPLATE.md: replaced. Instructions are HTML comments so they do not survive into the submitted description. Adds affected-area, configuration and generated-HLS sections, and an AI assistance disclosure with attestations the contributor makes about their own review. - ISSUE_TEMPLATE: converted to YAML issue forms, giving a chooser with bug report, performance issue, feature request, plan/RFC and documentation categories, plus discussion links for questions and ideas. Forms ask for the version, frontend, backend and io_type that reports usually omit. - AGENTS.md: the same expectations in a form AI agents read, kept short so harnesses that inject it into every request can afford it. --- .github/ISSUE_TEMPLATE/bug_report.md | 44 ----- .github/ISSUE_TEMPLATE/bug_report.yml | 123 +++++++++++++ .github/ISSUE_TEMPLATE/config.yml | 7 +- .github/ISSUE_TEMPLATE/documentation.yml | 47 +++++ .github/ISSUE_TEMPLATE/feature_request.md | 28 --- .github/ISSUE_TEMPLATE/feature_request.yml | 64 +++++++ .github/ISSUE_TEMPLATE/performance_issue.yml | 96 ++++++++++ .github/ISSUE_TEMPLATE/rfc.yml | 73 ++++++++ .github/PULL_REQUEST_TEMPLATE.md | 178 ++++++++++++++++--- AGENTS.md | 65 +++++++ CONTRIBUTING.md | 21 ++- MANIFEST.in | 2 +- 12 files changed, 643 insertions(+), 105 deletions(-) delete mode 100644 .github/ISSUE_TEMPLATE/bug_report.md create mode 100644 .github/ISSUE_TEMPLATE/bug_report.yml create mode 100644 .github/ISSUE_TEMPLATE/documentation.yml delete mode 100644 .github/ISSUE_TEMPLATE/feature_request.md create mode 100644 .github/ISSUE_TEMPLATE/feature_request.yml create mode 100644 .github/ISSUE_TEMPLATE/performance_issue.yml create mode 100644 .github/ISSUE_TEMPLATE/rfc.yml create mode 100644 AGENTS.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md deleted file mode 100644 index d0aa96a65b..0000000000 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ /dev/null @@ -1,44 +0,0 @@ ---- -name: Bug report -about: Something isn't working as expected -title: '' -labels: bug -assignees: '' - ---- - - -## Prerequisites -Please make sure to check off these prerequisites before submitting a bug report. -- [ ] Test that the bug appears on the current version of the master branch. Make sure to include the commit hash of the commit you checked out. -- [ ] Check that the issue hasn't already been reported, by checking the currently open issues. -- [ ] If there are steps to reproduce the problem, make sure to write them down below. -- [ ] If relevant, please include the hls4ml project files, which were created directly before and/or after the bug. - -## Quick summary -Please give a brief and concise description of the bug. - -## Details -Please add to the following sections to describe the bug as accurately as possible. - -### Steps to Reproduce -Add what needs to be done to reproduce the bug. Add *commented* code examples and make sure to include the original model files / code, and the commit hash you are working on. - -1. Clone the hls4ml repository -2. Checkout the master branch, with commit hash: [...] -3. Run conversion [...] on model file with code [...] -4. [Further steps ...] - -### Expected behavior -Please add a brief description of what you expected to happen. - -### Actual behavior -Describe what actually happens instead. - -## Optional - -### Possible fix -If you already know where the issue stems from, or you have a hint please let us know. - -### Additional context -Add any other context about the problem here. diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml new file mode 100644 index 0000000000..b1cf37e6af --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -0,0 +1,123 @@ +name: Bug report +description: Something in hls4ml is not working as expected +labels: ["bug"] +body: + - type: markdown + attributes: + value: | + Thanks for taking the time to file a bug report. + + Before filing, please check that the problem still occurs on the current `main` branch, and search + the [open issues](https://github.com/fastmachinelearning/hls4ml/issues) to see whether it has already + been reported. For questions and general problems that are probably not bugs, use + [Discussions](https://github.com/fastmachinelearning/hls4ml/discussions/categories/general) instead. + + - type: textarea + id: summary + attributes: + label: Quick summary + description: A brief and concise description of the bug. + validations: + required: true + + - type: input + id: version + attributes: + label: hls4ml version + description: Release number, or the commit hash if you are working from a checkout. + placeholder: "1.1.0, or commit abc1234" + validations: + required: true + + - type: dropdown + id: frontend + attributes: + label: Frontend + description: How the model enters hls4ml. + options: + - Keras v3 + - Keras v2 + - PyTorch + - ONNX / QONNX + - Other or not applicable + validations: + required: true + + - type: dropdown + id: backend + attributes: + label: Backend + options: + - Vitis + - Vivado + - VivadoAccelerator + - Quartus + - oneAPI + - Catapult + - Libero + - SymbolicExpression + - Other or not applicable + validations: + required: true + + - type: dropdown + id: io_type + attributes: + label: io_type + options: + - io_parallel + - io_stream + - Both + - Not applicable + validations: + required: true + + - type: input + id: tool_version + attributes: + label: HLS tool and version + description: Only if the bug involves synthesis. Write "not applicable" if it does not. + placeholder: "Vitis HLS 2024.1" + + - type: textarea + id: reproducer + attributes: + label: Steps to reproduce + description: | + The smallest script that shows the problem, plus how you ran it. Include the model definition + inline where possible, rather than attaching a large file. + render: python + validations: + required: true + + - type: textarea + id: expected + attributes: + label: Expected behaviour + validations: + required: true + + - type: textarea + id: actual + attributes: + label: Actual behaviour + description: Include the full error message or the incorrect output, not a paraphrase. + validations: + required: true + + - type: textarea + id: context + attributes: + label: Possible fix or additional context + description: Optional. If you know where the problem comes from, or have a hint, tell us here. + + - type: checkboxes + id: ai_disclosure + attributes: + label: AI assistance + description: | + See the [contributing guidelines](https://github.com/fastmachinelearning/hls4ml/blob/main/CONTRIBUTING.md#use-of-ai-tools). + Anything quoted here must come from a run you actually performed. + options: + - label: An AI tool helped produce this report, and I have checked its content myself. + required: false diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml index 907ac6db49..e931be128a 100644 --- a/.github/ISSUE_TEMPLATE/config.yml +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -1,5 +1,8 @@ blank_issues_enabled: false contact_links: - - name: Talk and engage with the comunity + - name: Question or general problem url: https://github.com/fastmachinelearning/hls4ml/discussions/categories/general - about: Check out the GitHub discusisons page for hls4ml. This is the best way to get in touch with us. In particular, if you have a question about hls4ml or a general problem that is likely not a bug. + about: Ask on the discussions page. This is the best way to get in touch with us, in particular for a question about hls4ml or a problem that is likely not a bug. + - name: Propose an idea + url: https://github.com/fastmachinelearning/hls4ml/discussions/categories/ideas + about: Float a feature idea with the community before opening a feature request or an RFC. diff --git a/.github/ISSUE_TEMPLATE/documentation.yml b/.github/ISSUE_TEMPLATE/documentation.yml new file mode 100644 index 0000000000..2950cb7dec --- /dev/null +++ b/.github/ISSUE_TEMPLATE/documentation.yml @@ -0,0 +1,47 @@ +name: Documentation issue +description: Something in the documentation is wrong, missing or unclear +labels: ["documentation"] +body: + - type: markdown + attributes: + value: | + This covers the documentation at [hls4ml.org](https://fastmachinelearning.org/hls4ml/), the + docstrings, and the guidance files in the repository. Corrections are welcome as pull requests too — + you do not have to file an issue first. + + - type: input + id: location + attributes: + label: Where + description: Page URL, file path, or the function whose docstring is affected. + validations: + required: true + + - type: dropdown + id: kind + attributes: + label: Kind of problem + options: + - Incorrect — it says something that is not true + - Missing — the topic is not documented + - Unclear — correct but hard to follow + - Outdated — describes behaviour that has since changed + validations: + required: true + + - type: textarea + id: details + attributes: + label: Details + description: What it says now, and what it should say instead. + validations: + required: true + + - type: checkboxes + id: ai_disclosure + attributes: + label: AI assistance + description: See the [contributing guidelines](https://github.com/fastmachinelearning/hls4ml/blob/main/CONTRIBUTING.md#use-of-ai-tools). + options: + - label: An AI tool helped produce this report, and I have checked its content myself. + required: false diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md deleted file mode 100644 index 1739f9d99f..0000000000 --- a/.github/ISSUE_TEMPLATE/feature_request.md +++ /dev/null @@ -1,28 +0,0 @@ ---- -name: Feature request -about: Suggest an idea for hls4ml -title: '' -labels: enhancement -assignees: '' - ---- - -## Prerequisites -Please talk to us before creating a new feature request. So that you can check that the idea is not already in active development. - -You can present your idea over here at the GitHub discussions page for hls4ml: https://github.com/fastmachinelearning/hls4ml/discussions/categories/ideas - -Even if an idea is already being worked on you can still create a feature request, -if you would like to open a discussion about the feature or want to contribute to it. - -## Details -Please add to the following sections to describe the feature as accurately as possible. - -### New behavior -Please add a brief and concise description of what you would like to happen in hls4ml in the future. - -### Motivation -Please tell us why this feature is important to the community. - -### Parts of hls4ml being affected -Please describe which parts of hls4ml would be affected by this feature. diff --git a/.github/ISSUE_TEMPLATE/feature_request.yml b/.github/ISSUE_TEMPLATE/feature_request.yml new file mode 100644 index 0000000000..9f85d35c91 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.yml @@ -0,0 +1,64 @@ +name: Feature request +description: Suggest a new capability for hls4ml +labels: ["enhancement"] +body: + - type: markdown + attributes: + value: | + Please consider raising the idea in + [Discussions](https://github.com/fastmachinelearning/hls4ml/discussions/categories/ideas) first, so + you can find out whether it is already being worked on. A feature request is still welcome afterwards, + as the place to track the work. + + If you are proposing a design rather than requesting a capability — something with a migration path, + a new configuration surface, or changes across several components — use the Plan / RFC template + instead. + + - type: textarea + id: problem + attributes: + label: Problem + description: What can you not do today, and why does it matter for real hls4ml users? + validations: + required: true + + - type: textarea + id: proposal + attributes: + label: Proposed behaviour + description: | + What hls4ml should do instead. If it adds a user-facing surface — a config key, a Strategy, an + `io_type`, an API argument — name it here. + validations: + required: true + + - type: textarea + id: scope + attributes: + label: Parts of hls4ml affected + description: Frontends, backends and components you expect this to touch. + validations: + required: true + + - type: textarea + id: alternatives + attributes: + label: Alternatives considered + description: Optional. Workarounds you tried, and why they are not enough. + + - type: checkboxes + id: contribution + attributes: + label: Contribution + options: + - label: I am willing to work on this myself. + required: false + + - type: checkboxes + id: ai_disclosure + attributes: + label: AI assistance + description: See the [contributing guidelines](https://github.com/fastmachinelearning/hls4ml/blob/main/CONTRIBUTING.md#use-of-ai-tools). + options: + - label: An AI tool helped produce this request, and I have checked its content myself. + required: false diff --git a/.github/ISSUE_TEMPLATE/performance_issue.yml b/.github/ISSUE_TEMPLATE/performance_issue.yml new file mode 100644 index 0000000000..268757295b --- /dev/null +++ b/.github/ISSUE_TEMPLATE/performance_issue.yml @@ -0,0 +1,96 @@ +name: Performance or resource usage issue +description: A model synthesizes, but is slower or larger than expected +labels: ["performance"] +body: + - type: markdown + attributes: + value: | + Use this for latency, initiation interval, resource usage or clock frequency that is worse than you + expect. The generated design has to build for this to be actionable — if it does not, file a bug + report instead. + + Please give numbers rather than impressions, and say which stage they come from. C synthesis reports + estimates; co-simulation gives true latency; logic synthesis gives true resources. + + - type: textarea + id: summary + attributes: + label: Quick summary + description: What is worse than expected, and compared to what? + validations: + required: true + + - type: input + id: version + attributes: + label: hls4ml version + placeholder: "1.1.0, or commit abc1234" + validations: + required: true + + - type: dropdown + id: backend + attributes: + label: Backend + options: + - Vitis + - Vivado + - VivadoAccelerator + - Quartus + - oneAPI + - Catapult + - Libero + - Other + validations: + required: true + + - type: input + id: tool_version + attributes: + label: HLS tool and version + placeholder: "Vitis HLS 2024.1" + validations: + required: true + + - type: input + id: part + attributes: + label: Part and clock period + placeholder: "xcvu13p-flga2577-2-e, 5 ns" + validations: + required: true + + - type: textarea + id: config + attributes: + label: Model and configuration + description: | + Layer shapes, precision, `io_type`, Strategy and reuse factor — ideally the script that produced + the project. + render: python + validations: + required: true + + - type: textarea + id: numbers + attributes: + label: Numbers observed and expected + description: | + Latency, initiation interval, LUT, FF, DSP, BRAM and achieved clock, as applicable. + value: | + | Measurement | Observed | Expected | Source (csynth / cosim / logic synth) | + |---|---|---|---| + | | | | | + validations: + required: true + + - type: checkboxes + id: ai_disclosure + attributes: + label: AI assistance + description: | + See the [contributing guidelines](https://github.com/fastmachinelearning/hls4ml/blob/main/CONTRIBUTING.md#use-of-ai-tools). + All numbers above must come from runs you actually performed. + options: + - label: An AI tool helped produce this report, and I have checked its content myself. + required: false diff --git a/.github/ISSUE_TEMPLATE/rfc.yml b/.github/ISSUE_TEMPLATE/rfc.yml new file mode 100644 index 0000000000..56e0ee860b --- /dev/null +++ b/.github/ISSUE_TEMPLATE/rfc.yml @@ -0,0 +1,73 @@ +name: Plan / RFC +description: Propose a design before implementing it +labels: ["rfc"] +title: "[RFC] " +body: + - type: markdown + attributes: + value: | + Use this for changes worth agreeing on before code exists: a new backend or frontend, a change to the + intermediate representation or optimizer machinery, a new configuration axis, or anything that + changes behaviour for existing users. + + The contributing guidelines ask you to discuss new functionality with us first. This is that + discussion, in a form we can track and refer back to. + + - type: textarea + id: motivation + attributes: + label: Motivation + description: The problem this solves, and who has it. Include the current behaviour you find inadequate. + validations: + required: true + + - type: textarea + id: design + attributes: + label: Proposed design + description: | + How it would work. Name the user-facing surface (config keys, API, generated code) and the internal + mechanism (passes, templates, types) as concretely as you can. + validations: + required: true + + - type: textarea + id: scope + attributes: + label: Components affected + description: | + Frontends, backends and parts of the tree. Say explicitly what happens on the backends that do not + support this, if it is not universal. + validations: + required: true + + - type: textarea + id: compatibility + attributes: + label: Compatibility and migration + description: | + Does existing user code, configuration or generated output change? If so, what do users have to do? + Write "no change" if there is none. + validations: + required: true + + - type: textarea + id: alternatives + attributes: + label: Alternatives considered + description: Designs you rejected and why. A rejected alternative is often the most useful part of an RFC. + + - type: textarea + id: plan + attributes: + label: Implementation plan + description: Optional. How this would land — in one pull request or several, and in what order. + + - type: checkboxes + id: ai_disclosure + attributes: + label: AI assistance + description: See the [contributing guidelines](https://github.com/fastmachinelearning/hls4ml/blob/main/CONTRIBUTING.md#use-of-ai-tools). + options: + - label: An AI tool helped produce this proposal, and I have checked its content myself. + required: false diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index e7565c2e11..af8a14f8bb 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,38 +1,164 @@ -# Description +## Description -> :memo: Please include a summary of the change. -> -> * Please also include relevant motivation and context. -> * List any dependencies that are required for this change. + -For a new feature or function, please create an issue first to discuss it -with us before submitting a pull request. +Fixes # -Note: Please delete options that are not relevant. +## Type of change -- [ ] Bug fix (non-breaking change that fixes an issue) -- [ ] Documentation update -- [ ] New feature (non-breaking change which adds functionality) -- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) -- [ ] A new research paper code implementation -- [ ] Other (Specify) +- [ ] Bug fix (non-breaking) +- [ ] New feature (non-breaking) +- [ ] Breaking change (existing configs, APIs or generated code behave differently) +- [ ] New frontend / backend +- [ ] New layer / operator support +- [ ] New configuration option — a new `io_type`, Strategy, or config attribute +- [ ] Research paper implementation +- [ ] Documentation +- [ ] Build, CI or tooling +- [ ] Refactor / cleanup (no functional change) + +## Affected areas + + + +**Backends:** +- [ ] +- [ ] Backend-independent (core IR, optimizer, `hls4ml.model`) + +**Frontends:** +- [ ] +- [ ] Not related to a frontend + +**Components:** +- [ ] IR (layers + model graph) / optimizer passes +- [ ] C++ templates / HLS sources under `hls4ml/templates/` +- [ ] Profiling, reporting or the CLI +- [ ] Packaging, build or CI + +## Configurations affected and exercised + + + +| Backend | `io_type` | Strategy | Verified | +|---|---|---|---| +| | | | | +| | | | | + +**New configuration axis introduced by this PR** (if any): + +## Impact on generated HLS + + + +- [ ] This PR **does not change** the generated HLS for existing models. +- [ ] This PR **changes** the generated HLS. Numbers below. + +**Numerical behaviour:** + +| Model / test | Backend & version | Part | Latency (cycles) | II | LUT | FF | DSP | BRAM | +|---|---|---|---|---|---|---|---|---| +| before | | | | | | | | | +| after | | | | | | | | | + + ## Tests -> :memo: Please describe the tests that you ran to verify your changes. -> -> * Provide instructions so we can reproduce. -> * Please also list any relevant details for your test configuration. + + +**Test configuration** (OS, Python, ML framework version, HLS tool version): -**Test Configuration**: +## Breaking changes and migration + + + +**What breaks:** + +**How users migrate:** + +## AI assistance disclosure + + + +- [ ] **None** — no AI tool was used. +- [ ] **Assisted** — completion, refactoring, docstrings, tests; design and code are mine. +- [ ] **Substantial** — significant AI-generated portions, reviewed and edited by me. +- [ ] **Agentic** — produced largely end-to-end by an AI agent from my prompts. + +Tool(s) and model(s): + +Where it was used: + +If anything other than *None* is ticked, confirm all of the following: + +- [ ] **I am the author of this contribution and take full responsibility for + it.** I have read and understood every line and can explain and defend it + in review. +- [ ] I verified the generated code against real hls4ml and HLS semantics — + no invented APIs, config keys, pragmas or citations. +- [ ] All numbers, logs and test results quoted in this PR come from runs I + actually performed, not from agent output. +- [ ] I have the right to submit this work under the project's licence, and to + my knowledge it does not reproduce third-party code that would conflict + with it. +- [ ] No AI tool is credited as an author in any commit in this branch. + + ## Checklist -- [ ] I have read the [guidelines for contributing](https://github.com/fastmachinelearning/hls4ml/blob/main/CONTRIBUTING.md). -- [ ] I have commented my code, particularly in hard-to-understand areas. -- [ ] I have made corresponding changes to the documentation. -- [ ] My changes generate no new warnings. -- [ ] I have installed and run `pre-commit` on the files I edited or added. -- [ ] I have added tests that prove my fix is effective or that my feature works. +**Required:** +- [ ] I have read the [contributing guidelines](https://github.com/fastmachinelearning/hls4ml/blob/main/CONTRIBUTING.md). +- [ ] I installed and ran `pre-commit` on the files I edited. +- [ ] I added tests under `test/pytest` covering this change (a bug fix should + add a test that fails on `main` and passes here). +- [ ] I self-reviewed the full diff and it contains no leftover debug code, + commented-out blocks or unrelated changes. +- [ ] The AI assistance disclosure above is complete and accurate. + +**If applicable:** +- [ ] Documentation under `docs/` updated. +- [ ] No new build or synthesis warnings. +- [ ] Public API / config schema changes are documented. + +## Release note + + + +```release-note + +``` diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000000..82eb19e9b5 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,65 @@ +# Working on hls4ml as an AI agent + +This file is for AI coding agents. Humans should read [CONTRIBUTING.md](CONTRIBUTING.md), which this file +follows; where the two appear to disagree, CONTRIBUTING.md wins. + +hls4ml compiles trained neural networks into HLS projects for FPGAs. The Python package is in `hls4ml/`, its +C++ kernel sources in `hls4ml/templates/`, tests in `test/pytest/`, documentation in `docs/`. + +Conversion is a pipeline, and almost every change belongs to exactly one stage of it: + +``` +frontend converter -> model graph -> optimizer passes -> code templates -> writer +hls4ml/converters/ hls4ml/model/ hls4ml/model/optimizer/, hls4ml/backends/*/passes/ hls4ml/writer/ +``` + +## Ground rules + +**Verify, do not assume.** hls4ml has several backends and they do not share conventions. A pragma, config +field, C++ class or attribute that exists for one backend may not exist for another. Read the file you are +about to rely on. Never invent an API, configuration key, pragma or citation. + +**Numbers come from runs you performed.** Do not quote latency, resource usage or accuracy that you did not +measure. If you did not run something, say so plainly. C synthesis reports estimates; co-simulation gives +true latency; logic synthesis gives true resource usage. Do not present an estimate as a measurement. + +**Match the surrounding code.** Comment density, naming and idiom should look like the file you are editing. +Do not add narration, banner comments, or explanations of what the next line does. Generated verbosity is a +common reason contributions are sent back. + +**Stay inside the requested scope.** Do not reformat unrelated code, rename things you were not asked to +rename, or fix unrelated problems in the same change. If you notice something else worth doing, mention it +rather than doing it. + +**Do not add dependencies casually.** The runtime dependency list is deliberately small and most extras are +optional. A new hard dependency needs a reason, and the default answer is an optional extra. + +## Authorship and disclosure + +Do not add `Co-authored-by` or any similar trailer crediting an AI tool in a commit message. Authorship +carries copyright, which a tool cannot hold, and such trailers distort contributor statistics. Some harnesses +add them automatically; do not. + +Disclosure belongs in the pull request description, which has a section for it. Fill in the tool and model +honestly, and state where assistance was used. + +The pull request template contains attestations that a human contributor makes about their own review and +rights. **Do not tick those on the human's behalf.** Fill in the factual parts, leave the attestations for +the person opening the pull request, and tell them what remains to be confirmed. + +## Before proposing a change + +- Run `pre-commit run --files ` and commit what it changes. It formats Python and C++ and will + reject the pull request otherwise. +- Add or update a test under `test/pytest/`. For a bug fix, the test should fail without the fix. +- Check the whole diff yourself, hunk by hunk, before presenting it. Remove debug output, commented-out code + and stray changes. +- Do not commit generated HLS projects, model files or logs. +- New functionality is discussed before it is built. If there is no issue for it, propose one rather than + opening a large unsolicited pull request. + +## Reporting back + +State what you actually did, including what you could not do. If a test fails, say so and show the output. If +you skipped a step, say which. Do not describe work as complete until it is, and do not summarize an intended +change as though it had been made. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 678eacd11d..c700676755 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -21,6 +21,10 @@ Consult [GitHub Help](https://help.github.com/articles/about-pull-requests/) for 1. Install and run `pre-commit` on the files that you have edited. 1. Submit the pull request to the [main](https://github.com/fastmachinelearning/hls4ml) branch. +Pull requests are **squash-merged**, so the individual commits on your branch are not preserved. +You do not need to curate them, but the pull request title and description become the permanent record of +the change in the project history, so please write them with that in mind. + ## Code Reviews We will review your contribution and, if any additional fixes or modifications are necessary, may provide feedback to guide you. @@ -28,13 +32,22 @@ When accepted, your pull request will be merged to the repository. ## Use of AI tools -Usage of AI tools in development for hls4ml is generally allowed. However, we require all contributers to adhere to the following guidelines: +Usage of AI tools in development for hls4ml is generally allowed. However, we require all contributors to adhere to the following guidelines: -- Contributed code must still be you own original work. It is your responsibility to make sure that the generated code is compatible with the [hls4ml license](LICENSE.txt), [these Contributor Guidelines](CONTRIBUTING.md), and it doesn't violate the lincense of either the AI tool or any third-party license obligations. -- The AI tool name and version must be disclosed in the PR description. +- Contributed code must still be your own original work. It is your responsibility to make sure that the generated code is compatible with the [hls4ml license](LICENSE), [these Contributor Guidelines](CONTRIBUTING.md), and that it doesn't violate the license of either the AI tool or any third-party license obligations. +- The AI tool name and version must be disclosed in the pull request description. The pull request template has a section for this. - Ensure you have reviewed and fully understand the generated code and be prepared to explain the reasoning behind it during review. - AI coding agents tend to be very verbose. Please review generated code and especially comments and trim unnecessary clutter. -- Do not spam the repository with issues and pull request for problems that have no likelihood of appearing in real world applications. Focus instead on fixing or improving hls4ml for real users. +- Numbers, logs and test results quoted in a pull request or issue must come from runs you actually performed, not from tool output that was never executed. +- Do not spam the repository with issues and pull requests for problems that have no likelihood of appearing in real world applications. Focus instead on fixing or improving hls4ml for real users. + +Do not credit an AI tool as an author of a commit. Authorship carries copyright, which a tool cannot hold, and +such trailers distort contributor statistics. Some assistants add `Co-authored-by` trailers automatically; +remove them before opening the pull request. Disclosure belongs in the pull request description, where a +reviewer will read it. + +If you use an AI agent to work on hls4ml, point it at [`AGENTS.md`](AGENTS.md) in the repository root. It +states the same expectations in a form agents read, and it will save you review comments. Submissions that appear unreviewed or copied directly from an AI tool without proper understanding may be requested to be revised or declined. diff --git a/MANIFEST.in b/MANIFEST.in index e3ee5ded3c..cf06df0be9 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,4 +1,4 @@ -include LICENSE README.md CONTRIBUTING.md CITATION.cff pyproject.toml .clang-format +include LICENSE README.md CONTRIBUTING.md AGENTS.md CITATION.cff pyproject.toml .clang-format graft example-models graft test graft contrib From 33aa26e259de5d077dfa01e8785c79d7ac144c55 Mon Sep 17 00:00:00 2001 From: Vladimir Loncar Date: Sun, 16 Aug 2026 15:25:49 +0200 Subject: [PATCH 3/4] Add agent-facing documentation under .agents/ --- .agents/README.md | 52 ++++++++ .agents/agent_adapters.py | 152 +++++++++++++++++++++ .agents/architecture-map.md | 185 ++++++++++++++++++++++++++ .agents/contributing-changes.md | 166 +++++++++++++++++++++++ .agents/evaluating-implementations.md | 123 +++++++++++++++++ .agents/frontends.md | 137 +++++++++++++++++++ .agents/kernels.md | 164 +++++++++++++++++++++++ .agents/local-setup.template.md | 49 +++++++ .agents/new-backend.md | 110 +++++++++++++++ .agents/optimizer-passes.md | 172 ++++++++++++++++++++++++ .agents/precision-and-debugging.md | 76 +++++++++++ .agents/reporting-issues.md | 118 ++++++++++++++++ .agents/running-hls4ml.md | 109 +++++++++++++++ .agents/toolchain-access.md | 113 ++++++++++++++++ .gitignore | 6 + AGENTS.md | 19 +++ test/pytest/test_agent_docs.py | 94 +++++++++++++ 17 files changed, 1845 insertions(+) create mode 100644 .agents/README.md create mode 100644 .agents/agent_adapters.py create mode 100644 .agents/architecture-map.md create mode 100644 .agents/contributing-changes.md create mode 100644 .agents/evaluating-implementations.md create mode 100644 .agents/frontends.md create mode 100644 .agents/kernels.md create mode 100644 .agents/local-setup.template.md create mode 100644 .agents/new-backend.md create mode 100644 .agents/optimizer-passes.md create mode 100644 .agents/precision-and-debugging.md create mode 100644 .agents/reporting-issues.md create mode 100644 .agents/running-hls4ml.md create mode 100644 .agents/toolchain-access.md create mode 100644 test/pytest/test_agent_docs.py diff --git a/.agents/README.md b/.agents/README.md new file mode 100644 index 0000000000..454ba0d3b9 --- /dev/null +++ b/.agents/README.md @@ -0,0 +1,52 @@ +# Agent documentation + +Working notes about how hls4ml is built, written to be read by an AI coding assistant and by anyone new to +the codebase. [AGENTS.md](../AGENTS.md) in the repository root is the short version that agents read first; +these documents are the detail it points to. + +They are not a substitute for [CONTRIBUTING.md](../CONTRIBUTING.md), which states the rules contributions +have to follow. Where the two appear to disagree, CONTRIBUTING.md wins. + +## The documents + +| Document | Read it when | +| --- | --- | +| [running-hls4ml.md](running-hls4ml.md) | you need to convert a model, run `predict()`, or check that a change works | +| [architecture-map.md](architecture-map.md) | before changing anything in the Python tree — what each stage owns and which file to open | +| [frontends.md](frontends.md) | adding support for a layer or operator, or a model fails to parse | +| [optimizer-passes.md](optimizer-passes.md) | changing the graph, adding a Strategy, a layer initializer or a config attribute | +| [kernels.md](kernels.md) | writing or modifying the C++ compute kernels | +| [precision-and-debugging.md](precision-and-debugging.md) | choosing fixed-point types, or the numerical result is wrong | +| [evaluating-implementations.md](evaluating-implementations.md) | claiming one implementation is faster or smaller than another | +| [new-backend.md](new-backend.md) | standing up a backend for a new toolchain | +| [toolchain-access.md](toolchain-access.md) | synthesizing, choosing a tool version, or a build cannot find its tool | +| [contributing-changes.md](contributing-changes.md) | shaping a change, and again before opening a pull request | +| [reporting-issues.md](reporting-issues.md) | reporting a bug, a performance problem, or proposing a feature | +| [local-setup.template.md](local-setup.template.md) | recording how your own machine is set up | + +`frontends.md` ends with an end-to-end checklist for adding a layer, which sequences the others. + +## Using these with your assistant + +Each document carries a short front matter block with a `name`, a `description` saying when it applies, and +`globs` listing the paths it covers. Different assistants consume that differently, so the files are kept in +one neutral place and adapted rather than duplicated: + +``` +python .agents/agent_adapters.py --list # what can be generated +python .agents/agent_adapters.py claude cursor # generate those views +``` + +Generated views are ignored by git. Nothing prevents you from pointing your assistant at `.agents/` directly — +the files are plain Markdown and the front matter is harmless. + +## Keeping them true + +These describe mechanisms that change. Two rules keep them from rotting: + +- A change to the machinery a document describes updates that document in the same pull request. +- `test/pytest/test_agent_docs.py` checks that every repository path mentioned in these files still exists. + It runs in the normal test suite; a renamed module makes it fail. + +Statements should be checkable. Prefer naming the file that proves a claim over asserting it, and do not +record numbers from a specific machine or a specific project — the point is what stays true. diff --git a/.agents/agent_adapters.py b/.agents/agent_adapters.py new file mode 100644 index 0000000000..082fc745c4 --- /dev/null +++ b/.agents/agent_adapters.py @@ -0,0 +1,152 @@ +#!/usr/bin/env python3 +"""Generate assistant-specific views of the documents in .agents/. + +The documents in .agents/ are the single source of truth. Assistants disagree about where such files live and +what metadata they carry, so this script writes copies in the layout each one expects. Generated output is +git-ignored; edit .agents/ and regenerate. + + python .agents/agent_adapters.py --list + python .agents/agent_adapters.py claude cursor + python .agents/agent_adapters.py --all --clean + +Adding an adapter is a matter of writing one function and adding it to ADAPTERS. Please do not add +assistant-specific content to the source documents themselves. +""" + +from __future__ import annotations + +import argparse +import re +import shutil +from pathlib import Path + +SOURCE = Path(__file__).resolve().parent +REPO = SOURCE.parent +SKIP = {'README.md', 'local-setup.template.md'} + + +def read_docs() -> list[dict]: + """Return the parsed front matter and body of every source document.""" + docs = [] + for path in sorted(SOURCE.glob('*.md')): + if path.name in SKIP: + continue + text = path.read_text() + match = re.match(r'^---\n(.*?)\n---\n(.*)$', text, re.S) + if not match: + raise SystemExit(f'{path}: missing front matter') + front, body = match.group(1), match.group(2) + + name = re.search(r'^name:\s*(.+)$', front, re.M) + description = re.search(r'^description:\s*>-\n((?:\s{2,}.*\n?)+)', front, re.M) + globs = re.findall(r'^\s+-\s*"(.+)"$', front, re.M) + if not name or not description: + raise SystemExit(f'{path}: front matter needs a name and a description') + + docs.append( + { + 'stem': path.stem, + 'name': name.group(1).strip(), + 'description': ' '.join(line.strip() for line in description.group(1).splitlines()), + 'globs': globs, + 'body': body.lstrip('\n'), + 'path': path, + } + ) + return docs + + +def adapt_claude(docs: list[dict]) -> Path: + """Claude Code: .claude/skills//SKILL.md, dispatched by description.""" + out = REPO / '.claude' / 'skills' + for doc in docs: + directory = out / doc['name'] + directory.mkdir(parents=True, exist_ok=True) + body = doc['body'].replace('](', '](../../../.agents/') + front = f'---\nname: {doc["name"]}\ndescription: {doc["description"]}\n---\n\n' + (directory / 'SKILL.md').write_text(front + body) + return out + + +def adapt_cursor(docs: list[dict]) -> Path: + """Cursor: .cursor/rules/.mdc, activated by glob.""" + out = REPO / '.cursor' / 'rules' + out.mkdir(parents=True, exist_ok=True) + for doc in docs: + globs = ', '.join(doc['globs']) + front = f'---\ndescription: {doc["description"]}\nglobs: {globs}\nalwaysApply: false\n---\n\n' + (out / f'{doc["stem"]}.mdc').write_text(front + doc['body']) + return out + + +def adapt_copilot(docs: list[dict]) -> Path: + """GitHub Copilot: .github/instructions/.instructions.md, applied by path.""" + out = REPO / '.github' / 'instructions' + out.mkdir(parents=True, exist_ok=True) + for doc in docs: + apply_to = ','.join(doc['globs']) or '**' + front = f"---\napplyTo: '{apply_to}'\n---\n\n" + (out / f'{doc["stem"]}.instructions.md').write_text(front + f'\n\n' + doc['body']) + return out + + +def adapt_plain(docs: list[dict]) -> Path: + """A single concatenated file, for assistants that take one document.""" + out = REPO / 'agent-docs.md' + parts = ['# hls4ml agent documentation\n', '\n'] + for doc in docs: + parts.append(f'\n\n---\n\n\n\n{doc["body"]}') + out.write_text(''.join(parts)) + return out + + +ADAPTERS = { + 'claude': adapt_claude, + 'cursor': adapt_cursor, + 'copilot': adapt_copilot, + 'plain': adapt_plain, +} + + +def main() -> None: + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument('adapters', nargs='*', metavar='ADAPTER', help=f'any of: {", ".join(ADAPTERS)}') + parser.add_argument('--all', action='store_true', help='generate every view') + parser.add_argument('--list', action='store_true', help='list the available views and exit') + parser.add_argument('--clean', action='store_true', help='remove generated output first') + args = parser.parse_args() + + if args.list: + for name, func in ADAPTERS.items(): + print(f'{name:10s} {func.__doc__.splitlines()[0]}') + return + + selected = list(ADAPTERS) if args.all else args.adapters + if not selected: + parser.error('name at least one adapter, or pass --all (see --list)') + unknown = [name for name in selected if name not in ADAPTERS] + if unknown: + parser.error(f'unknown adapter(s): {", ".join(unknown)} (see --list)') + + docs = read_docs() + for name in selected: + if args.clean: + target = ( + REPO + / { + 'claude': '.claude/skills', + 'cursor': '.cursor/rules', + 'copilot': '.github/instructions', + 'plain': 'agent-docs.md', + }[name] + ) + if target.is_dir(): + shutil.rmtree(target) + elif target.exists(): + target.unlink() + written = ADAPTERS[name](docs) + print(f'{name}: wrote {len(docs)} documents to {written.relative_to(REPO)}') + + +if __name__ == '__main__': + main() diff --git a/.agents/architecture-map.md b/.agents/architecture-map.md new file mode 100644 index 0000000000..533985ac86 --- /dev/null +++ b/.agents/architecture-map.md @@ -0,0 +1,185 @@ +--- +name: architecture-map +description: >- + Map of how hls4ml turns a trained model into an HLS project — the graph and attribute system, the flow / + optimizer-pass machinery, the C++ templates, and the writer. Read this BEFORE changing anything inside the + hls4ml Python tree, or when you need to find which file controls a given piece of generated code. Covers + what each stage owns, how a config value reaches a C++ struct field, and where to look for a given task. +globs: + - "hls4ml/model/**" + - "hls4ml/backends/**" + - "hls4ml/writer/**" +--- + +# How hls4ml is put together + +All paths below are relative to the package directory `hls4ml/hls4ml/` inside the repo checkout. + +The conversion is a five-stage pipeline. Every stage only communicates with the next through **layer +attributes**, so almost every change you will make is "set a different attribute" or "read an attribute and +emit different text". + +``` +frontend converter → ModelGraph → flows of optimizer passes → templates → writer +(converters/) (model/graph.py) (model/optimizer/, backends/*/passes/) (writer/) +``` + +1. **Frontend** (`converters/`) parses Keras / PyTorch / ONNX into a plain list of layer dictionaries, whose + `class_name` selects the `Layer` subclass — see [**frontends**](frontends.md). +2. **`ModelGraph.from_layer_list`** builds the graph of `Layer` nodes (`model/layers.py`). +3. **`model.apply_flow(backend.get_default_flow())`** runs the registered optimizer passes in order. +4. Template passes fill in the code attributes of each node — in the Vivado family `config_cpp` and + `function_cpp`; these strings *are* the generated C++. +5. The backend's **writer** (`writer/_writer.py`) writes the project directory. + +## The graph and its nodes + +- `model.graph` is an `OrderedDict` of `name -> Layer`. Insertion order is the emission order. +- Nodes are connected by **tensor name strings**, not object references: `node.inputs` and `node.outputs` are + lists of names. `node.get_input_node()`, `node.get_input_variable()`, `node.get_output_variable()` resolve + them through `model.output_vars`. When you rewire the graph, you rewrite these name lists. +- `Layer.initialize()` is where a layer declares its output shape and weights + (`add_output_variable`, `add_weights`, `add_bias`). `Dense.initialize` is the shortest useful example. + +## The attribute system — the single source of truth + +Everything about a layer lives in `layer.attributes` (`model/attributes.py`), a dict with side effects: + +- Storing a `TensorVariable` registers it as a model output variable and also sets `result_t`. +- Storing a `WeightVariable` named `weight` automatically creates the type attribute `weight_t`. +- Four filtered views exist: `layer.weights`, `layer.variables`, `layer.types`, `layer.code`. +- `Layer.expected_attributes` is collected over the whole class hierarchy from each class's + `_expected_attributes` list. Anything expected but unset and without a default raises at construction. +- Attribute kinds: `Attribute`, `ConfigurableAttribute` (user may set it), `TypeAttribute` (name always ends + in `_t`), `ChoiceAttribute`, `WeightAttribute`, `CodeAttrubute` (spelling as in the source). + +**How a user config value reaches a layer:** `HLSConfig` (`model/graph.py`) reads the `HLSConfig` dict; in +`Layer.__init__` each key of the layer's config is converted from pascal case to snake case +(`ReuseFactor` -> `reuse_factor`), and any key ending in `_t` whose value is a string is converted into a +`NamedType` through the backend's `convert_precision_string`. So a user knob and a layer attribute are the +same thing under two spellings. + +## Flows and passes + +- A `Flow` (`model/flow/flow.py`) is a named list of pass names plus a `requires` list of other flows. + `apply_flow` walks requirements depth first and skips flows already applied. +- Three flows are registered by the core, without a backend prefix, in `model/optimizer/__init__.py`: + `parse_qonnx`, `convert` (frontend cleanups, ending with `infer_precision_types`), and `optimize`. Every + backend's `init_layers` requires the core `optimize`. Note the name collision: an unprefixed `optimize` in a + `requires` list is the core flow, while `vivado:optimize` is a different, backend-owned flow. +- Passes are registered globally in `optimizer_map`, prefixed with the backend name: + `vivado:distributed_arithmetic_codegen`. Backends discover them automatically — `Backend._init_file_optimizers` + (`backends/backend.py`) scans the `passes/` directory of **`[*self.__class__.__bases__, self.__class__]`**, + not the full method resolution order, and `_init_class_optimizers` picks up decorated backend methods. +- A registered pass that is in no flow never runs. +- `VitisBackend._register_flows` (`backends/vitis/vitis_backend.py`) is the best example to copy: it builds + its own flows and inserts them into a copy of `vivado:ip`'s requirement list at chosen positions. + +## Templates — where C++ text comes from + +`backends/template.py` defines two template base classes, both of which are optimizer passes: + +- `LayerConfigTemplate` sets the node's `config_cpp` attribute — the `struct configN { ... };` block. +- `FunctionCallTemplate` sets `function_cpp` and `include_header` — the one-line call in `myproject.cpp`. + +Both format a Python string against `node.attributes` plus a few extras (`_default_config_params` adds +`iotype`, `reuse`, `namespace`; `_default_function_params` adds `config`, `input_t`, `output_t`, `input`, +`output`). `backends/vivado/passes/core_templates.py` holds the Dense and activation templates and shows how +`strategy` selects the C++ kernel class name. + +Types and variables are converted from the backend-independent form into backend code form by that backend's +own `passes/transform_types.py`, using converters built on `backends/fpga/fpga_types.py`. Vivado, Quartus, +oneAPI, Catapult and Libero each have their own copy; Vitis has none and uses Vivado's through inheritance. +This pass is where the backend decides what a tensor becomes — an array with a partition pragma, an +`hls::stream`, or a pipe. io_type behaviour is decided there, not in the writer. + +A backend may add template kinds of its own by subclassing `Template` with a different `attribute_name`; see +`backends/oneapi/oneapi_template.py`. The writer then reads those extra attributes. + +## What is universal and what is only a convention + +Stages 1 to 3 above — the graph, the attribute system, flows and passes — are core and identical for every +backend. From stage 4 onward, backends diverge, and how much they follow the Vivado pattern varies: + +| Universal | Vivado/Vitis convention that others follow only partly | +| --- | --- | +| `ModelGraph`, `Layer`, attributes, `expected_attributes` | the `config_cpp` / `function_cpp` attribute pair as the only generated-code channel | +| flows, `register_flow`, pass discovery and `backend:` naming | `LayerConfigTemplate` and `FunctionCallTemplate` as the only template kinds | +| `Template` as the mechanism that produces code attributes | `nnet::` headers, the `strategy` enum, the `kernel` typedef, `nnet_utils/` layout | +| a registered `Writer` subclass that emits the project | the file names and directory layout listed below | +| variable and type converters applied by a `transform_types` pass | `hls::stream>` for io_stream, arrays plus partition pragmas for io_parallel | + +Concrete counter-examples in this tree: oneAPI defines two extra template kinds +(`StreamFunctionCallTemplate`, `TaskSequenceTemplate` in `backends/oneapi/oneapi_template.py`), carries data +in pipes with `pipe_name` rather than `hls::stream`, and keeps its headers under +`templates/oneapi/firmware/nnet_utils/`. Quartus and oneAPI have no `strategy` field and no kernel typedef. +The symbolic backend has no `nnet_utils` at all. + +Read as: the Python machinery is a contract, the C++ shape is a precedent. A backend may define its own +template kinds, its own variable types, and its own project layout; doing so is a supported extension, not a +workaround. + +## The writer (Vivado family) + +`VivadoWriter.write_hls` calls, in order: `write_project_dir`, `write_project_cpp`, `write_project_header`, +`write_weights`, `write_defines`, `write_parameters`, `write_test_bench`, `write_bridge`, +`write_build_script`, `write_nnet_utils`, `write_generated_code`, `write_yml`, `write_tar`. + +Resulting project layout: + +| Path | Contents | +| --- | --- | +| `firmware/myproject.cpp` | top function: stream declarations, `#pragma HLS DATAFLOW` or pipeline, one `function_cpp` line per layer | +| `firmware/myproject.h` | top function signature | +| `firmware/defines.h` | typedefs for every layer type and variable | +| `firmware/parameters.h` | every `config_cpp` struct, plus includes | +| `firmware/weights/w2.h, b2.h …` | weight arrays | +| `firmware/nnet_utils/` | the C++ kernel headers, copied from `templates//nnet_utils/` | +| `myproject_test.cpp`, `tb_data/` | C simulation testbench | +| `myproject_bridge.cpp` | the shared library used by `compile()` / `predict()` | +| `build_prj.tcl`, `project.tcl` | synthesis driver scripts | +| `hls4ml_config.yml` | the resolved configuration | + +The writer should stay simple: it copies text that passes and templates already produced. If you find +yourself adding model logic to the writer, it belongs in a pass instead. + +Three of those outputs are easy to forget when changing the project structure, and each has a consumer: + +- **The bridge** (`write_bridge`, from `templates//myproject_bridge.cpp`) is what `compile()` builds + and `predict()` calls through ctypes, and it also carries the trace-collection entry points used by + `trace()`. Change the top function's interface and the bridge must change with it, or Python-side + verification breaks while synthesis still succeeds. +- **The testbench** (`write_test_bench`, plus `tb_data/`) is what C simulation and co-simulation run. +- **The build scripts** (`write_build_script`) are copied from the backend's `templates/` directory and are + what `build()` ultimately invokes; the report that `build()` returns is parsed by `hls4ml/report/`. + +## Where to look for a given task + +| I want to change… | Open | +| --- | --- | +| what a layer's generated struct contains | `backends//passes/*_templates.py` | +| which C++ kernel is called | the same template's `format()` — in the Vivado family, its `strategy` branch | +| the compute itself | `templates//nnet_utils/nnet_*.h` | +| graph shape, layer fusion, layer removal | a pass in `backends//passes/` or `model/optimizer/passes/` | +| a new user-settable knob | a `ConfigurableAttribute` in the layer class or `_register_layer_attributes` | +| whether a tensor is an array or a stream | `passes/transform_types.py` plus `backends/fpga/fpga_types.py` | +| default precision, accumulator width | `model/optimizer/passes/infer_precision.py`, `HLSConfig.get_precision` | +| the order things run in | the backend's `_register_flows` | +| files written to disk | `writer/_writer.py` | + +## Reading order for a first session + +`model/layers.py` (the `Layer` base class and `Dense`), `model/attributes.py`, `backends/backend.py`, +`backends/template.py`, `backends/vivado/passes/core_templates.py` (Dense part only), then one generated +project's `parameters.h` and `myproject.cpp` side by side with the templates that produced them. That last +step is worth more than reading any further Python. + +Dense and the Vivado backend are used here because they are the shortest complete path, not because they are +representative. Once the mechanism is clear, read the files of the backend and layer family you are actually +changing: that backend's `_register_flows`, its `passes/`, its `transform_types.py`, its writer, and the +templates for your layer. Assume nothing carries over until you have seen it there. + +Related: [**frontends**](frontends.md) for getting a layer in and for the end-to-end checklist, +[**optimizer passes**](optimizer-passes.md) for writing the passes, [**kernels**](kernels.md) for the generated code and the +precision rules, [**running hls4ml**](running-hls4ml.md) for actually converting a model, [**new backend**](new-backend.md) for standing +up a backend of your own. diff --git a/.agents/contributing-changes.md b/.agents/contributing-changes.md new file mode 100644 index 0000000000..12300e6996 --- /dev/null +++ b/.agents/contributing-changes.md @@ -0,0 +1,166 @@ +--- +name: contributing-changes +description: >- + Shape, finish and submit a change to hls4ml — the scope rules that decide whether something is a pass, a + Strategy or a new backend, pre-commit, the test conventions, and how to fill in the pull request template + honestly. Use when deciding up front how a feature should be shaped, and again when the work is done and + the next step is opening a pull request. Includes the pre-submission cleanup sweep. +globs: + - "test/pytest/**" + - "CONTRIBUTING.md" +--- + +# Contributing a change to hls4ml + +Read [CONTRIBUTING.md](../CONTRIBUTING.md) and [AGENTS.md](../AGENTS.md) first; this document is the working +detail behind them. + +`main` moves quickly, so branch from an up-to-date `main` and rebase again before opening the pull request. +Pull requests are squash-merged, so commit granularity does not matter — the pull request title and +description become the permanent record. + +## Shape the change before writing it + +`CONTRIBUTING.md` asks for a GitHub issue or discussion first for anything non-trivial. Beyond that, the +scope rules that decide how a feature is received: + +- **Prefer a Strategy or a pass inside an existing backend over a new backend.** If the change could be + expressed as a strategy plus a kernel plus a few passes in Vitis or Vivado, that is what it should be. A + new backend is justified only by genuinely different structure, such as a different toolchain or a + different memory model. +- **Keep backend-specific code out of `hls4ml/model/`.** Anything that names a toolchain, a pragma, or an + FPGA part belongs in `hls4ml/backends//`. +- **Do not change defaults for existing users.** New attributes need defaults; new strategies must be opt-in. + A model that converted before must convert identically afterwards unless the change is explicitly a fix. +- **Fail loudly on unsupported combinations.** The patterns in the tree are a validation pass that warns and + corrects the attribute (`backends/vitis/passes/feature_check.py`), or a check at the point the attribute is + set that raises naming the conflict — as `VivadoBackend.init_dense` does when a strategy is combined with a + reuse factor it cannot support. +- **Every backend that shares the code path has to keep working.** Vitis inherits from Vivado, so a change in + `backends/vivado/` affects both. Changes to `model/` affect all backends including Quartus, oneAPI, + Catapult and Libero. + +## 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 run --files +``` + +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. + +## Tests + +`CONTRIBUTING.md` requires a unit test under `test/` for new functionality, and for a bug fix a test that +fails on `main` and passes on the branch. Tests live in `test/pytest/test_*.py`. + +One test is the floor, not the target. **Cover a reasonable set of the configurations your feature supports**, +because a feature that works in one configuration and silently misbehaves in another is the usual way +regressions reach users. Parametrize over the axes your change is actually keyed on: + +- `backend` — every backend the change reaches. A change under `backends/vivado/` reaches Vitis too. +- `io_type` — both `io_parallel` and `io_stream` when the feature exists in both; for several layer families + these are separate implementations rather than a shared one. +- `strategy`, `reuse_factor`, precision, layer shapes — whichever your feature is keyed on, including the + boundary cases. For reuse factor those are the values around `n_in`. + +The point is coverage of what the feature claims to support, not a combinatorial sweep. State the axes you +chose in the pull request, and say which combinations you deliberately left out. + +`test/pytest/test_keras_api.py` and `conftest.py` show the conventions to copy: + +- Build a small model in the test, convert it, `compile()`, `predict()`, and compare against the framework + prediction with `np.testing.assert_allclose` at a tolerance appropriate to the precision. Where the test is + about an implementation rather than about quantization, use a wide type so the tolerance can be tight. +- Use the `test_case_id` fixture for the output directory so parametrized runs do not collide: + `output_dir = str(test_root_path / test_case_id)`. +- Assert on attributes as well as numbers where the feature is a configuration path — that a strategy written + in several spellings resolves to the expected attribute, that a layer count or shape is what you expect. +- CI runs one job per test file, generated by `test/pytest/generate_ci_yaml.py`. Keeping a new feature's + tests in their own file keeps the job self-contained. + +### Synthesis in tests + +Synthesis is **optional but recommended**: C simulation proves the arithmetic, and only a build proves the +generated code is synthesizable at all. It is off by default and must stay that way — the `synthesis_config` +fixture reads the `RUN_SYNTHESIS` environment variable and the per-backend tool versions, and +`run_synthesis_test` in `test/pytest/synthesis_helpers.py` returns immediately when it is disabled. Never +make a test depend on a toolchain being present. + +**Keep a synthesized test as small as it can be while still exercising the feature.** Build time grows quickly +with model size, and these runs are on shared CI. One or two layers with small shapes is enough to catch what +this level catches: code that does not build, a pragma that does not apply, a scheduling failure. Depth and +realism belong in C simulation tests, which cost seconds. `test_keras_api.py::test_dense` is the model to +follow — a single `Dense(2)` on a one-element input, with an activation. + +`run_synthesis_test` compares the resulting report against a stored baseline under +`test/pytest/baselines///`, so a synthesized test needs a baseline file committed for the +backends and tool versions it runs on. + +## Documentation + +- User-facing features get a page under `docs/advanced/` (see `da.rst`, `auto.rst`, `bramfactor.rst` for the + expected length and tone) and a link from the relevant index. +- New layer attributes should carry a `description=` taken from `hls4ml/utils/attribute_descriptions.py`; + `docs/attr_doc_gen.py` turns those into the attribute tables in the documentation. +- The PR description itself must document the change and the expected behaviour — that is item one of the + contribution guidelines. + +## The cleanup sweep + +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. +- **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 + new code does not use — unused headers, unreferenced passes, config fields no kernel reads. +- **Unrelated changes.** No reformatting sweeps mixed into a functional change, no drive-by edits to files + the change does not need, no `.gitignore` or editor settings that only matter on your machine. +- **New dependencies.** The runtime dependency list is deliberately small and nearly everything else is an + optional extra in `pyproject.toml`. A new hard dependency needs a justification in the pull request; the + default answer is an extra. +- **Local paths and machine specifics.** No absolute paths from your filesystem, tool install locations or + scratch directories in code, tests or documentation. +- **Generated artifacts.** Do not commit HLS projects, build directories, reports or model files. Several + patterns are ignored already, but a project written to a different name is not. +- **C++ formatting.** Run pre-commit on edited headers too, not only Python; `p-clang-format` handles them. +- **Invented interfaces.** Every API, config key, attribute and pragma you used must exist — check it in the + tree rather than trusting recall. This is what the template's attestation is about. +- **TODOs.** Either fix it, or open an issue and reference it. A bare `TODO` in a contribution is noise. + +## Filling in the pull request + +The template asks for specific things, and half-filling it is worse than leaving a section out: + +- **Describe the change, not the process.** What it does, why, and what a user notices. No narration of how + the work went. +- **Configuration table:** one row per configuration you actually exercised. Where an axis does not apply to + a backend, write `n/a` rather than guessing a plausible value. +- **Verification column and the generated-HLS table:** enter only what you ran. A blank cell is honest; an + invented number is not. C synthesis estimates are labelled as such, never as measured latency or resources. +- **AI assistance disclosure:** fill in the tool, the model and where it was used. +- **Do not tick the attestations.** Those are statements the human contributor makes about their own review, + rights and understanding. An agent filling in the rest of the template should leave them blank and tell the + contributor which ones remain. +- **Do not credit an AI tool as a commit author.** No `Co-authored-by` trailers for tools, in any commit on + the branch. +- **Release note:** one user-facing sentence, or the word `none`. + +## Before opening the pull request + +1. Rebased on current `main`, and the branch contains only intended changes. +2. Correctness verified by C simulation against the reference model, on every io_type and backend touched. +3. Performance claims backed by co-simulation for latency and logic synthesis for resources, not by C + synthesis estimates. See [evaluating implementations](evaluating-implementations.md). +4. Tests added and passing, including the existing tests for the layers that share the code path being + changed. Say which command you ran. +5. `pre-commit` clean on all edited files. +6. The cleanup sweep above, done against the actual diff, hunk by hunk. +7. The template filled in honestly, with the attestations left for the human contributor. diff --git a/.agents/evaluating-implementations.md b/.agents/evaluating-implementations.md new file mode 100644 index 0000000000..18d40fb66f --- /dev/null +++ b/.agents/evaluating-implementations.md @@ -0,0 +1,123 @@ +--- +name: evaluating-implementations +description: >- + Fairly benchmark a new or modified hls4ml compute kernel / Strategy / layer implementation against the + baseline it wants to replace. Use whenever deciding go/no-go on a kernel change, a new strategy, an + upstream PR, or any claim of the form "X is faster/smaller than Y" in hls4ml or its HLS backends. Covers + building a fair comparison, the sweep matrix (shapes, io_type, reuse_factor), dependent-layer fallout, + synthesis artifacts that fake wins, and reporting a defensible verdict. +globs: + - "hls4ml/templates/**" + - "hls4ml/backends/**" +--- + +# Evaluating an hls4ml implementation (kernel / strategy / layer) + +A benchmark is worth only as much as its fairness. Treat the first result as wrong until you have shown that +the two implementations differ in **exactly one** variable. + +## The cardinal rule: compare through real hls4ml, not a standalone harness + +Do not write a hand-rolled C++ and C-synthesis harness that calls the two kernels and compares the reports. +It silently handicaps one side. The usual mechanism: a harness passes weights as a function argument, so the +tool infers a memory interface with one or two ports, while real hls4ml emits them as partitioned constant +arrays. Whichever kernel issues more weight reads per cycle is then throttled by the harness rather than by +its own design, and the difference can be an order of magnitude. Similar distortions come from a missing +top-level pragma, a different interface mode, or weights that are constant on one side and not the other. + +Instead, integrate **both** implementations as first-class options inside hls4ml — a new Strategy, or a +kernel selected by the layer config — and generate one project per variant with the same backend, io_type, +precision, part and clock. hls4ml then emits identical scaffolding around each kernel, so the kernel is the +only difference. Confirm this by diffing the two generated `firmware/` directories: everything except the +intended change should be identical. + +**Calibrate before sweeping.** Check at least one configuration against a number you trust — the same layer +built through the unmodified path, or a published figure. If a single point is far from expectation, stop and +find the cause. A sweep built on an unfair setup produces a large, self-consistent, wrong answer. + +## Verify correctness before looking at quality of results + +For every configuration: convert, `compile()`, `predict()`, and compare against the reference model. The +maximum absolute error should be consistent with the precision in use — of order `1e-3` for +`ap_fixed<16,6>`. A kernel that synthesizes small and fast but computes the wrong answer is not a win. + +Sign flips and transposes hide behind symmetric test data. Use non-square shapes and asymmetric weights and +inputs, so an index error cannot cancel itself out. + +## The sweep matrix — square shapes at the default reuse factor are not enough + +Rankings flip across regimes, so vary every axis: + +- **Shapes:** `n_in == n_out`, `n_in > n_out`, `n_in < n_out`, powers of two, and odd sizes. Odd and + non-square shapes expose padding and divisibility assumptions. +- **io_type:** both `io_parallel` and `io_stream`. They wrap kernels differently, and for some layer families + io_stream is a separate implementation rather than a wrapper. +- **reuse_factor:** the full valid range, not the default alone. Only certain values are valid; respect the + backend's `_validate_reuse_factor`. Report results grouped by regime, because that is where rankings + diverge: `rf < n_in`, `rf == n_in` (the common default — a loss here usually decides the verdict), and + `rf >> n_in` (large layers folded to save multipliers). +- **Precision:** at least two widths. The balance between logic and multiplier use moves with bit width. +- **Backend:** if the change touches shared code, every backend that reaches it. + +When the two implementations expose different parallelism knobs, match resources rather than parameters: +choose settings that give both the same multiplier count, then compare logic, registers, clock frequency and +latency at that operating point. + +## Establish the weight storage model first + +Before drawing any conclusion about resources, know how the design under test is meant to hold its weights, +because the answer changes what a result means: + +- **Baked into the design as constants.** This is the default and, for most implementations and backends, the + intended deployment. The tool then replaces multipliers with fixed-coefficient logic wherever it can, and + multiplier counts fall. **That saving is real** — it is part of what the implementation delivers, not an + artifact — as long as the design ships with constant weights. +- **Held in memory, on-chip or off-chip, or reloadable at runtime.** Weights are read rather than folded into + the arithmetic, so strength reduction does not apply and the multipliers come back. hls4ml exposes part of + this today through `BramFactor`, which turns weights above a size threshold into a BRAM interface on the + Vivado, Vitis and Catapult backends; the default threshold is high enough that weights stay constant unless + you ask otherwise. Work aimed at off-chip or reloadable weights lives in this mode by design. + +The mistake is carrying a result across the boundary. A constant-weight comparison says nothing about a +design that must load weights at runtime, and vice versa. So state which mode you measured in, measure the +implementation in the mode it is actually aimed at, and if a claim is meant to hold in both, measure both. + +## Synthesis artifacts that fake a win + +- **Pipeline or dataflow directives not applied.** If a new strategy is not recognized by the pass that sets + the pipeline style, the kernel is left serial or fully unrolled and the reported cost describes the + scaffolding, not the design. A resource use far from what the parallelism setting implies is the signal. +- **Estimates instead of measurements.** C synthesis under-reports multi-layer io_stream latency for both + sides. Use it for relative comparison at matched conditions only. + +## Check the layers that share the algorithm + +A change to the matrix-vector code path is not local: the convolution family reuses it, and recurrent layers +call a dense-like kernel per gate. Before declaring a result, generate a layer from each dependent family +through the changed path and confirm correctness and cost there too. A win in one layer that regresses +another is not a win. + +## Deliverables + +- A results table with ratios per configuration (logic, registers, latency, clock frequency, multipliers), + averaged **within** each regime and never across regimes. State the convention explicitly, for example + that a ratio below one means smaller or faster. +- Plots of those ratios against size and regime. +- A short report covering: why the comparison is fair and which point was calibrated, what holds across all + configurations, the regime split, the artifacts discounted, and a one-line verdict naming the operating + point it applies to. "Wins in a narrow regime" is a legitimate verdict — say which regime, and what + adopting it would cost. + +## Anti-checklist + +1. Concluding from a standalone harness rather than from real hls4ml output. +2. Reporting square shapes at the default reuse factor and calling it a sweep. +3. Comparing under constant weights and claiming the result for a design that loads its weights, or the + reverse. +4. Forgetting the layer families that share the algorithm. +5. Averaging across regimes, which hides a loss in the most common one. +6. Reporting C synthesis estimates as final latency or resource numbers. + +Latency numbers belong to co-simulation and resource numbers to logic synthesis, at one fixed tool version +across the whole comparison. See [**toolchain access**](toolchain-access.md) for running those here, and +[**contributing changes**](contributing-changes.md) once a verdict says the change is worth proposing. diff --git a/.agents/frontends.md b/.agents/frontends.md new file mode 100644 index 0000000000..d0ffe6e6eb --- /dev/null +++ b/.agents/frontends.md @@ -0,0 +1,137 @@ +--- +name: frontends +description: >- + Teach hls4ml to parse a layer it does not know — the Keras v3 and v2, PyTorch and ONNX handler patterns, + registering the hls4ml Layer class, what the config helpers need, and the extension API for out-of-tree + layers. Use whenever the task is "add support for layer/op X", a model fails to convert with an unsupported + or unknown layer, or you need to know where a parsed attribute comes from. Ends with the end-to-end + checklist for adding a layer, which sequences the other hls4ml skills. +globs: + - "hls4ml/converters/**" + - "hls4ml/model/layers.py" + - "hls4ml/utils/config.py" +--- + +# Frontends: getting a layer into hls4ml + +Paths are relative to the package directory `hls4ml/hls4ml/`. + +A frontend turns a trained model into a **list of plain dictionaries**, one per hls4ml layer. Each dictionary +carries at least `name`, `class_name`, and the input/output tensor names; `class_name` selects the `Layer` +subclass through `model/layers.py`'s `layer_map`, and the remaining keys become that layer's attributes. +Everything downstream — passes, templates, writer — sees only the graph built from those dictionaries. + +## First decide: in-tree or extension API + +A layer of general interest belongs in the tree. A layer specific to one project can be added from outside it +with the **extension API**, which registers the same pieces at runtime without patching hls4ml: + +```python +hls4ml.converters.register_keras_v2_layer_handler('KReverse', parse_reverse_layer) +hls4ml.model.layers.register_layer('KReverse', HReverse) +backend.register_template(HReverseConfigTemplate) +backend.register_template(HReverseFunctionTemplate) +backend.register_source('/path/to/nnet_reverse.h') +``` + +`docs/advanced/extension.rst` walks a complete example, and `test/pytest/test_extensions.py` and +`test_extensions_pytorch.py` are runnable versions of it. The component list is the same either way, which +makes the extension example the best template for in-tree work too. + +## Keras v3 — the current path + +Handlers live in `converters/keras_v3/`. A handler is a class; defining it registers it, through the +metaclass, for every Keras class named in `handles`: + +```python +class DenseHandler(KerasV3LayerHandler): + handles = ('keras.src.layers.core.dense.Dense',) + + def handle(self, layer, in_tensors, out_tensors): + kernel = self.load_weight(layer, 'kernel') + bias = self.load_weight(layer, 'bias') if layer.use_bias else None + n_in, n_out = kernel.shape + return {'data_format': 'channels_last', 'weight_data': kernel, + 'bias_data': bias, 'n_out': n_out, 'n_in': n_in} +``` + +Things the base class does for you, worth knowing before writing a handler: + +- `handles` entries are **fully qualified Keras module paths**, not display names. Get them from the layer's + `__module__` and class name, not from the Keras docs. +- When `handle` returns a **single dict**, the base fills in `name`, `class_name`, `module`, + `input_keras_tensor_names`, `input_shape` and `output_keras_tensor_names`, and copies `epsilon`, + `use_bias`, `data_format` if the layer has them. Anything you return overrides those. +- When `handle` returns a **tuple of dicts** — one Keras layer becoming several hls4ml layers — none of that + is automatic: every dict must carry `name`, `class_name`, `input_keras_tensor_names` and + `output_keras_tensor_names` itself, and an assertion fires if one is missing. +- **A layer with an `activation` attribute is split automatically** into your layer plus a following + activation layer, with an intermediate tensor threaded between them. This is why one Keras `Dense(..., + activation='relu')` becomes two nodes in the graph. Softmax, hard sigmoid, leaky ReLU and ELU map to + dedicated classes with their parameters filled in; everything else becomes `Activation`. +- `self.load_weight(layer, 'kernel')` returns a NumPy array regardless of the Keras backend in use. +- Weight arrays go into the dictionary under the names the `Layer` class expects, conventionally + `weight_data` and `bias_data`, because `add_weights_variable` looks up `_data`. + +## Keras v2, PyTorch, ONNX + +Same idea, function-based instead of class-based: + +- **Keras v2** (`converters/keras_v2_to_hls.py`): a function decorated with `@keras_handler('ClassName')`, + taking the layer's serialized config and returning `(layer_dict, output_shape)`. `parse_default_keras_layer` + fills the common keys. Registration also works at runtime through + `register_keras_v2_layer_handler(name, func)`. Conversion tries v3 first and falls back to v2 unless + `allow_v2_fallback=False`, so a model may reach either path — when a layer parses in one and not the other, + this is why. +- **PyTorch** (`converters/pytorch_to_hls.py`): `@pytorch_handler('ClassName', ...)`. +- **ONNX** (`converters/onnx_to_hls.py`): `@onnx_handler('OpName', ...)`. + +Supporting a layer in one frontend does not support it in the others. Decide explicitly which frontends the +task covers, and say so. + +## Registering the hls4ml layer + +`register_layer(name, clazz)` in `model/layers.py` adds an entry to `layer_map`, keyed by the `class_name` +your handler emits. Without it two things break: `ModelGraph.make_node` cannot resolve the layer, and +`config_from_keras_model` raises a `KeyError` before conversion even starts, because it looks the class up to +discover which attributes are configurable. + +That lookup is also why the config helpers need nothing layer-specific: at `'type'` or `'name'` granularity +they walk the parsed list, take each layer class's `expected_attributes`, and emit a key for every attribute +marked configurable — a `TypeAttribute` becomes a `Precision` entry (`'auto'` when it has no default), a +`reuse_factor` becomes `ReuseFactor`, and the rest are copied from the parsed dictionary or their default. A +new layer therefore appears in the generated config automatically, provided its attributes are declared +properly. See [**optimizer passes**](optimizer-passes.md) for declaring them. + +## Adding a layer end to end + +The full path, with the skill that covers each step: + +1. In-tree or extension API — this skill. +2. Write the frontend handler for each frontend in scope — this skill. +3. Define the `Layer` subclass: `_expected_attributes` and `initialize()` (output shape, weights) — + [**architecture map**](architecture-map.md). +4. Register it in `layer_map` — this skill. +5. Add a branch to `model/optimizer/passes/infer_precision.py` if the layer's types should be inferable; + without one it falls to the default rule — [**optimizer passes**](optimizer-passes.md). +6. Backend initializer (`@layer_optimizer`) to set and validate attributes, plus any backend-specific + configurable attributes — [**optimizer passes**](optimizer-passes.md). +7. Config and function templates for each backend in scope — [**architecture map**](architecture-map.md). +8. The C++ kernel, for each io_type you claim to support — [**kernels**](kernels.md). +9. Convert, compile, predict against the reference model — [**running hls4ml**](running-hls4ml.md). +10. Synthesize — [**toolchain access**](toolchain-access.md). +11. Tests and pull request — [**contributing changes**](contributing-changes.md). + +Steps 3, 4, 7 and 8 are the ones that fail loudly. Steps 5 and 6 fail quietly: the layer converts and +computes, with types or a strategy nobody chose. + +## Anti-checklist + +- Do not assume the display name works in `handles`; it takes the fully qualified module path. +- Do not hand-fill the automatic keys when returning a single dict, and do not forget them when returning a + tuple. +- Do not be surprised by the extra activation node — check whether your layer should keep its activation + fused instead, and handle that in a pass rather than in the parser. +- Do not add a handler without registering the layer class; the failure appears in the config helper, far + from the cause. +- Do not claim support for a frontend you did not implement or test. diff --git a/.agents/kernels.md b/.agents/kernels.md new file mode 100644 index 0000000000..e80ecf9a6c --- /dev/null +++ b/.agents/kernels.md @@ -0,0 +1,164 @@ +--- +name: kernels +description: >- + Write or modify hls4ml's C++ compute kernels. Use whenever the task touches a backend's nnet_utils headers + or adds the kernel behind a Strategy. The kernel-contract sections describe the Vivado/Vitis family + convention, which other backends follow only partly; the closing section says what holds for a backend that + looks nothing like it. For the types flowing through these kernels, see precision-and-debugging.md. +globs: + - "hls4ml/templates/**" +--- + +# hls4ml compute kernels + +Paths are relative to the package directory `hls4ml/hls4ml/`. + +## Scope: how much of this is a convention, not a rule + +The C++ side is **not uniform across backends**. What follows describes the Vivado/Vitis convention, which +Libero follows closely and Catapult partly. Check the backend you are actually working on: + +| Backend | `strategy` in the config struct | `kernel` typedef dispatch | header location | Dense weight layout | +| --- | --- | --- | --- | --- | +| Vivado, Vitis, Libero | yes | yes | `templates//nnet_utils/` | native `weights[i*n_out+j]` | +| Catapult | yes | no (branches inside the header) | `templates//nnet_utils/` | native | +| Quartus, oneAPI | no | no (`dense_rf_gt` / `dense_rf_lt` chosen by reuse factor) | `templates//firmware/nnet_utils/` | padded and rounded, `reuse_factor_rounded * block_factor_rounded` | +| symbolic | no nnet_utils at all | | | | + +oneAPI further replaces `hls::stream` with pipes (variables carry `pipe_name`) and adds its own template +kinds in `backends/oneapi/oneapi_template.py`. A backend under development may share none of this. + +So: read the target backend's own `nnet_common.h` and `nnet_dense.h` before assuming any of the sections +below. If the backend has no such files, skip to "If your backend does not look like this" at the end. + +## The kernel contract (Vivado/Vitis family) + +Kernels live in `templates//nnet_utils/*.h` and are copied verbatim into +`/firmware/nnet_utils/` by the writer's `write_nnet_utils`. A backend can add a file from +elsewhere with `backend.register_source(abs_path, destination_dir='nnet_utils')`. + +Generated C++ per layer is: a config struct in `parameters.h`, and a one-line call in `myproject.cpp`. For +Dense: + +```cpp +struct config2 : nnet::dense_config { + static const unsigned n_in = 16; + static const unsigned n_out = 32; + static const unsigned strategy = nnet::resource; + typedef accum_default_t accum_t; + typedef weight2_t weight_t; + template using kernel = nnet::DenseResource_rf_leq_nin<...>; + template using product = nnet::product::mult; +}; +nnet::dense(input, layer2_out, w2, b2); +``` + +- `nnet::dense` in `nnet_dense.h` does nothing except dispatch: + `CONFIG_T::template kernel::dense(...)`. +- A kernel is a class deriving from `nnet::DenseKernel` in `nnet_function_stubs.h` with one static `dense` + method of the fixed signature `(data[n_in], res[n_out], weights[n_in*n_out], biases[n_out])`. + `Conv1DKernel` and `DepthwiseDenseKernel` follow the same pattern. +- A new kernel is therefore three things: the class in a header, a `dense_function` branch in the config + template (`backends/vivado/passes/core_templates.py`), and an `init_dense` branch that sets `strategy`. +- Dense weight layout in this family is `weights[i * n_out + j]` (input-major), exactly as the frontend + produced it. This is a per-backend decision, not a global one — Quartus and oneAPI pass a padded, rounded + layout instead. If your kernel wants a different layout, rearrange it in a pass, not at runtime. +- Never hardcode `ap_fixed` inside a kernel. Use `typename CONFIG_T::accum_t` for accumulators, + `CONFIG_T::template product::product(x, w)` for the multiply (this is what makes binary + and ternary weights work), and `cast(acc)` from `nnet_mult.h` for the output + conversion. +- Shared helpers in `nnet_common.h`: the `io_parallel` / `io_stream` and `latency` / `resource` / + `resource_unrolled` / `distributed_arithmetic` enums, `DIV_ROUNDUP`, `PRAGMA_DATA_PACK`, and the balanced + tree `reduce>` for summing a fully unrolled array. + +## io_parallel and io_stream + +Two io types with these two representations is a Vivado-family arrangement. oneAPI carries data in pipes +instead, and a new backend may define something else entirely. + +- **io_parallel:** tensors are plain C arrays, partitioned by a pragma chosen in `transform_types.py`. The + kernel signature above is used directly. +- **io_stream:** tensors are `hls::stream>` (see `nnet_types.h`). For Dense, the wrapper in + `nnet_dense_stream.h` reads the input stream into a local array (`data_prepare`), calls the same + array-based kernel, and writes the result out (`res_write`), so one kernel serves both io types. Conv and + pooling do not work this way: their io_stream form is a separate line-buffer implementation. +- **The trap in that wrapper:** it branches on `CONFIG_T::strategy`. Under `latency` it calls + `dense_latency_wrapper`, which carries `#pragma HLS PIPELINE II=CONFIG_T::reuse_factor` and therefore fully + unrolls whatever kernel is underneath. A dataflow-style kernel must reach the `resource` branch, or its + cost model is destroyed. When adding a strategy, check every place that compares against + `nnet::latency` or `nnet::resource`, including `backends/vivado/passes/pipeline_style.py`. + +## Dense is the simplest kernel, not a representative one + +Dense is used above because it is the shortest complete example. It is a poor model for most other work, +because it has one input, one weight set, a fixed shape relation, and an io_stream path that is only a +wrapper around the array kernel. Before working on another family, read that family's own headers and +template file; the list below says what each one adds that Dense does not show. + +| Family | Headers / templates | What is different | +| --- | --- | --- | +| Conv1D/2D | `nnet_conv*.h`, `nnet_conv_stream.h`, `convolution_templates.py` | io_stream has its own line-buffer implementation rather than a wrapper; per-layer C++ is **generated** by `im2col_codegen.py` and injected at the `// hls4ml insert code` marker in `nnet_code_gen.h` by `write_generated_code` | +| Separable / depthwise | `nnet_sepconv*.h`, `nnet_depthwise_product.h` | two weight sets and two chained kernels for one logical layer | +| Pooling | `nnet_pooling*.h`, `pooling_templates.py` | no weights; window arithmetic and padding dominate the config | +| Activations | `nnet_activation*.h`, activation part of `core_templates.py` | no weights; lookup tables sized by the `table_size` and `table_t` attributes registered in `FPGABackend._register_layer_attributes`; softmax additionally has the `fix_softmax_table_size` pass in `backends/fpga/passes/` | +| Merge, Concatenate | `nnet_merge*.h`, `merge_templates.py` | several inputs, so the template must resolve `get_input_variable(name)` per input rather than assuming `inputs[0]` | +| Recurrent (LSTM, GRU) | `nnet_recurrent.h`, `recurrent_templates.py` | separate `recurrent_reuse_factor` and `static` attributes, state carried across time steps, a nested Dense-like kernel per gate | +| Reshape, Flatten, Transpose | `nnet_transpose*.h`, `reshaping_templates.py` | may produce an `InplaceTensorVariable` — no data movement and no kernel call at all | +| Distributed arithmetic | `distributed_arithmetic.py`, `nnet_da_wrappers.h` | the kernel body itself is generated per layer from the weight values, so there is no fixed header to edit | +| Einsum, GarNet, SNN | `nnet_einsum*.h`, `nnet_garnet.h`, `nnet_snn*.h` | their own config vocabulary; useful precedents when a layer does not fit the matrix-vector shape | + +Two consequences worth carrying into any kernel work: a change to the matrix-vector code path affects the +conv family too, because they share it; and a family whose io_stream form is a separate implementation +(conv, pooling) must be validated in both io types separately, since fixing one does not fix the other. + +## Pragmas that matter + +| Pragma | Effect | Cost when misused | +| --- | --- | --- | +| `PIPELINE II=n` | one result every n cycles; unrolls everything inside | full unroll of a large loop nest, huge LUT and DSP use | +| `UNROLL factor=k` | k copies of the loop body | silent resource blow-up if k is derived from a layer dimension | +| `ARRAY_PARTITION complete` | every element in its own register | fine for tens of elements, not for thousands | +| `ARRAY_RESHAPE cyclic factor=k` | k-wide access per cycle from one memory | mismatch with the loop's access pattern gives no speedup | +| `DATAFLOW` | run stages concurrently | only helps if stages communicate through streams or single-writer arrays | +| `INLINE` / `INLINE recursive` | remove the function boundary | large inlined bodies slow synthesis considerably | + +Aim for one clear parallelism knob per kernel (hls4ml's is `reuse_factor`) and make the pragmas derive from +it, so the cost is predictable across shapes. + +## If your backend does not look like this + +For a backend under development, or one of the non-Vivado backends, treat everything above as one worked +example rather than a specification. Only the Python-side contract is fixed, and it is short: + +- Each node must end up with the attributes the backend's own writer reads. For the Vivado family those are + `config_cpp` and `function_cpp`; oneAPI adds `stream_function_cpp` and `task_sequence_cpp` through its own + `Template` subclasses in `backends/oneapi/oneapi_template.py`. **Defining new template kinds is a supported + extension point**, not a workaround — subclass `Template` with your own `attribute_name`. +- The backend must supply variable and type converters that turn `TensorVariable` and `NamedType` into + whatever its language needs, applied by its own `transform_types` pass. Arrays and `hls::stream` are the + Vivado answer; pipes are the oneAPI answer; a new backend may have a third. +- The writer is registered per backend and may emit any layout it wants. There is no required project + structure, only the one each writer creates. +- Everything else — the `nnet::` namespace, `nnet_common.h`, the strategy enum, the `kernel` typedef, the + `dense_config` base struct, io_parallel and io_stream as the only two io types — is a convention of the + Vivado family that other backends adopt as far as it suits them. + +When shared machinery does not fit a new backend, the intended response is for that backend to define its +own version, not to bend the design to what the shared code allows. Say out loud which convention you are +diverging from and why. + +## Anti-checklist + +- Do not write a standalone C++ harness that calls the kernel directly to measure it. Passing weights as an + argument makes the tool infer a limited-port memory interface, throttling whichever kernel reads more + weights per cycle. Compare through real hls4ml, where the scaffolding around both kernels is identical; + see [**evaluating implementations**](evaluating-implementations.md). +- Do not carry a Vivado/Vitis convention into another backend without checking it there. The `strategy` + field, the `kernel` typedef, the header path and the weight layout all differ across backends already. +- Do not generalize from Dense. Check whether the family you are touching has a separate io_stream + implementation, generated per-layer code, several inputs, or several weight sets. +- Do not assume the io_stream wrapper will leave your kernel alone; check which strategy branch it takes. +- Do not hardcode a type, a shape, or a parallelism factor inside a kernel; everything comes from `CONFIG_T`. + +For the precision rules these kernels obey, and for reading the project they generate, see +[precision and debugging](precision-and-debugging.md). diff --git a/.agents/local-setup.template.md b/.agents/local-setup.template.md new file mode 100644 index 0000000000..34edf4c740 --- /dev/null +++ b/.agents/local-setup.template.md @@ -0,0 +1,49 @@ +--- +name: local-setup +description: >- + Site-specific setup for this machine or group — interpreter, installed extras, how the HLS toolchain is + reached, and where builds are run. Copy this template, fill it in, and keep it out of version control unless + your group shares one machine. The other documents in this directory deliberately contain no local paths. +globs: + - "**" +--- + +# Local setup + + + +## Python environment + +- Interpreter: +- Optional extras installed: +- Frontend backend variable: +- How to add a package here: +- Directories to avoid running Python from: + +## HLS toolchain + +- Tools and versions available: +- How they are reached: +- Wrapper scripts: +- Container binds needed: +- Per-run state: + +## Running builds + +- Default part and clock used for comparisons: +- Where to put build scratch: +- How long a typical build takes: +- Job submission, if builds go to a batch system: + +## Anything else that surprises a newcomer + + diff --git a/.agents/new-backend.md b/.agents/new-backend.md new file mode 100644 index 0000000000..84d5b98693 --- /dev/null +++ b/.agents/new-backend.md @@ -0,0 +1,110 @@ +--- +name: new-backend +description: >- + Create a new in-tree hls4ml FPGA backend — its Backend subclass, Writer, passes, and code templates — by + forking an existing backend and pruning it to what is actually supported. Use when standing up a backend + for a new toolchain or a variant of an existing one. Covers deciding whether a new backend is warranted, + what to subclass, the pass discovery and namespacing rules, registration, and how to avoid carrying dead + copied code. +globs: + - "hls4ml/backends/**" + - "hls4ml/writer/**" +--- + +# Standing up a new hls4ml backend + +Paths are relative to the package directory `hls4ml/hls4ml/`. + +## First, decide whether it should be a backend at all + +A new backend is warranted by a different toolchain, a different code language or memory model, or a project +structure the existing writers cannot express. It is not warranted by a better kernel or a different +scheduling idea: those are a **Strategy plus passes** inside an existing backend, which is far less code to +maintain and far easier to get reviewed upstream. + +The test: if the change could be rolled into an existing backend without contorting it, it is not a new +backend. Answer this before writing code, because the answer determines everything below. + +## Fork, then prune + +hls4ml has no scaffolding generator, and copying an existing backend is the sanctioned starting point. The +trap is that a fork silently carries the entire feature surface of its source. A backend that supports two +layer types should not ship headers, passes and writer paths for twenty. Prune to what is supported, and copy +the next piece fresh from the source backend when you add each new feature. + +## What to subclass + +- **Backend:** inherit from `FPGABackend` (`backends/fpga/fpga_backend.py`), not from `VivadoBackend` or + `VitisBackend`, unless you genuinely want their whole pass set. `FPGABackend` provides the shared FPGA + plumbing without toolchain-specific passes. +- **Writer:** there is no `FPGAWriter`. Writers subclass either `Writer` (`writer/writers.py`) or an existing + backend's writer. For a backend that is not a variant of another, subclass `Writer` directly. +- **Types, passes, templates:** fork the closest existing backend's `*_types.py`, `passes/` and its + `templates/` directory, renaming throughout. + +You are not obliged to keep the source backend's C++ conventions. The `nnet::` namespace, the strategy enum, +the kernel typedef dispatch and the `hls::stream` representation are conventions of the Vivado family that +other backends in the tree follow only partly. Defining your own variable types, your own `Template` +subclasses and your own project layout is a supported extension, not a workaround. Say explicitly which +convention you are diverging from and why. + +## Pass discovery and namespacing — the rule that governs everything + +`Backend._init_file_optimizers` walks `[*self.__class__.__bases__, self.__class__]` — your direct bases plus +your own class, **not** the full method resolution order — and registers each `passes/` directory it finds, +prefixed with `self.name.lower() + ':'`. + +Consequences to design around: + +- `class YourBackend(FPGABackend)` registers `backends/fpga/passes/` and `backends/yourbackend/passes/` as + `yourbackend:*`. It does not pull in another backend's passes. Anything you need from an existing backend + must be copied into your own `passes/`. +- Because only direct bases are scanned, a deep inheritance chain does not stack passes. Keep `FPGABackend` as + the base and put everything else in your own directory. +- Cross-backend pass names resolve globally, but referencing one creates a dependency on that backend's flows. + Prefer your own copies. + +## Registration + +- `backends/__init__.py`: `register_backend('YourName', YourBackend)`. +- `writer/__init__.py`: register the writer. + +## The backend class — minimum viable shape + +- `_register_flows`: register the flows you support. Five names appear in every FPGA backend — + `init_layers`, `specific_types`, `apply_templates`, `write` and `ip` — and following them makes your + backend legible to anyone who knows another one. `optimize`, `streaming` and `quantization` are optional + additions; Libero registers only the five and is the smallest example to copy. A minimal backend may leave + shared FPGA passes unwired — the warning about optimizers not in any flow is informational, not an error. +- `create_initial_config`: defaults for part, io_type and clock, plus your own knobs. +- `build()`: the synthesis entry point for your toolchain. +- `init_` methods decorated with `@layer_optimizer`: set and validate the attributes your kernels need. + Watch for interactions with any wrapper your io_stream path uses — in the Vivado family the stream wrapper + branches on `strategy` and will force-pipeline a kernel that reaches the wrong branch. + +## Prune aggressively + +After forking, delete what the backend does not support and trim what remains: + +- **Kernel headers:** keep only those the supported layers use. Trim a kept header to the configuration + struct and the dispatch for the kernels you actually ship. +- **Passes:** keep the template passes for supported layers, the type transformation pass, and the pipeline + style pass. Delete passes for layers and strategies you do not ship. +- **Writer:** remove the paths for features you do not support, and make sure the writer copies the build + scripts you do use. +- **Configuration structs:** emit only the fields the kernel reads. Fields inherited from a base config + struct and never used mislead readers into thinking the kernel honours them. + +## Verify end to end + +Gate on a real convert, write, compile, predict and synthesize run of a small model in your target io_type, +comparing against the reference framework at a tolerance consistent with the precision. Confirm that each +custom attribute actually threads from the configuration through to the generated code — read the generated +file, do not assume. + +hls4ml does not clean `output_dir` on re-conversion. Files from an earlier run persist, so inspect only a +freshly created or deleted-and-recreated directory. + +Related: [**architecture map**](architecture-map.md) and [**optimizer passes**](optimizer-passes.md) for the machinery you are wiring into, +[**evaluating implementations**](evaluating-implementations.md) for benchmarking the kernels, [**toolchain access**](toolchain-access.md) for reaching the +toolchain your `build()` invokes. diff --git a/.agents/optimizer-passes.md b/.agents/optimizer-passes.md new file mode 100644 index 0000000000..8865b214f0 --- /dev/null +++ b/.agents/optimizer-passes.md @@ -0,0 +1,172 @@ +--- +name: optimizer-passes +description: >- + Write or modify an hls4ml optimizer pass, a Strategy, a layer initializer, a validation check, or a new + user-settable config attribute. Use whenever the task is "make hls4ml do X to the graph / set X on a layer + / expose X in the config" rather than changing the C++ kernel itself. Covers the pass types, automatic + discovery, flow registration and ordering, the transform return value, graph edit APIs, and the traps that + cause a pass to silently never run or to loop forever. +globs: + - "hls4ml/model/optimizer/**" + - "hls4ml/backends/*/passes/**" +--- + +# Writing hls4ml optimizer passes + +Paths are relative to the package directory `hls4ml/hls4ml/`. Passes are the only sanctioned way to change a +model between parsing and writing. If a change can be expressed as a pass, it should be. + +## Pick the right pass type + +All of these live in `model/optimizer/optimizer.py`. + +| Type | Runs on | Use for | +| --- | --- | --- | +| `OptimizerPass` | every node where `match(node)` is true | the normal case: rewriting, checking, annotating a layer | +| `GlobalOptimizerPass` | every node (`match` always true) | sweeping work such as type conversion (`transform_types.py`) | +| `ModelOptimizerPass` | the whole model once, `transform(model)` | decisions needing the full graph, such as chain detection | +| `ConfigurableOptimizerPass` | as `OptimizerPass`, plus `configure(**kwargs)` | passes with options, e.g. `InferPrecisionTypes` | +| `Template` subclasses | matching nodes | generating C++ text; see the architecture-map skill | +| `@layer_optimizer(LayerClass)` on a backend method | instances of that layer class | backend-specific layer initialization, e.g. `init_dense` | + +A backend method decorated with `@layer_optimizer(Dense)` becomes a `LayerOptimizerPass` named after the +method (`vitis:init_dense`). Layer initializers are sorted by the length of the layer class's method +resolution order, so `init_base_layer` runs before `init_dense`. + +## Discovery and naming + +- **File passes:** any class in `backends//passes/*.py` that subclasses `OptimizerPass` and is + *defined in that module* (not imported into it) is registered automatically. Alternatively, define a + function `register_(backend)` in the module and it is called instead, which lets you + register several passes explicitly. +- The registered name is `snake_case(ClassName)` unless the class sets `name`, and it is always prefixed with + the backend name in lower case: `DistributedArithmeticCodegen` in `backends/vivado/passes/` becomes + `vivado:distributed_arithmetic_codegen`. +- Discovery walks `[*self.__class__.__bases__, self.__class__]`, so a `VitisBackend(VivadoBackend)` gets + `backends/vivado/passes/` and `backends/vitis/passes/`, but a backend two levels down does **not** inherit + its grandparent's passes. +- Model-level passes in `model/optimizer/passes/` are registered without a backend prefix and are available + to every backend. + +**A pass that is registered but not in a flow never runs.** This is the most common reason new pass code +appears to do nothing. + +## Flow registration and ordering + +Register a flow with `register_flow(name, optimizers, requires=[...], backend=self.name)` inside the +backend's `_register_flows`. To insert your flow at a specific point in an existing pipeline, copy the +requirement list of the flow you are extending and insert into it — this is what Vitis does: + +```python +validation_flow = register_flow('validation', validation_passes, requires=['vivado:init_layers'], backend=self.name) +ip_flow_requirements = get_flow('vivado:ip').requires.copy() +ip_flow_requirements.insert(ip_flow_requirements.index('vivado:init_layers'), validation_flow) +self._default_flow = register_flow('ip', None, requires=ip_flow_requirements, backend=self.name) +``` + +Backend flow names that appear in every FPGA backend, each under its own prefix: `init_layers`, +`specific_types`, `apply_templates`, `write` and `ip`. Others are optional and vary — Vivado, Quartus, oneAPI +and Catapult add `optimize`, `streaming` and `quantization`, while Libero registers only the five above and +Vitis adds `validation` on top of the ones it inherits from Vivado. Libero is the useful minimal example. + +Do not confuse a backend's own `:optimize` with the unprefixed core `optimize` flow registered in +`model/optimizer/__init__.py`. Every backend's `init_layers` requires the **core** one. + +Ordering rules worth memorizing: + +- Anything that reads or sets **types** must run before `*:specific_types` (which runs `transform_types` and + turns tensors into arrays or streams). After that pass the variables are backend C++ objects. +- Anything that changes **which C++ kernel is selected** must run before `*:apply_templates`. +- Precision inference is the pass `infer_precision_types` + (`model/optimizer/passes/infer_precision.py`). It runs at the end of the core `convert` flow, and some + backends list it again in their own `optimize` flow to catch types that became inferable later. Before it, + types may still be `UnspecifiedPrecisionType`; do not read a final bit width earlier than that. + +## The transform return value + +`optimize_model` loops over the passes repeatedly: + +- Returning **`True` means "the graph changed"**, which aborts the current sweep and restarts from the first + pass. Return `True` only after adding or removing nodes. +- Returning `False` (or `None`) means "I edited attributes only, keep going". Template passes deliberately + return `False`. +- Returning `True` when nothing actually changed makes conversion hang forever. If a conversion never + finishes, suspect this first. +- A `ModelOptimizerPass` is applied at most once per flow, and only recorded as applied if it returns a + truthy value. + +## Editing the graph + +`ModelGraph` (`model/graph.py`) provides `make_node`, `insert_node`, `remove_node`, `replace_node`, +`split_node`. Constraints that are enforced with exceptions or assertions: + +- `make_node(kind, name, attributes, inputs, outputs=None)` creates a detached node; the layer type must be + in `layer_map` (`register_layer(name, clazz)` in `model/layers.py`), and the backend wraps the class to add + its own expected attributes. +- `insert_node` accepts only nodes with a single input; pass `before=` when the predecessor has several + consumers. +- `remove_node` requires at most one input and one output, and asserts that the input and output shapes have + the same number of elements. +- `replace_node` requires the same number of inputs and outputs as the node being replaced. + +All of these rewrite the tensor-name lists of neighbouring nodes. After any of them, return `True`. + +## Adding a user-settable attribute + +Two options: + +1. **In the layer class** — add a `ConfigurableAttribute` to `_expected_attributes` in `model/layers.py`. + Applies to every backend. +2. **In the backend** — append to `self.attribute_map[LayerClass]` in the backend's + `_register_layer_attributes`. `FPGABackend.create_layer_class` then builds a subclass of the layer that + carries these extra expected attributes, so the knob exists only for that backend. This is the right place + for anything toolchain-specific. + +Give every attribute a default. A `ConfigurableAttribute` with no default and no user value raises during +layer construction, which breaks every existing model. + +`infer_precision_types` is the other place a new layer class needs an entry. `_infer_precision` dispatches on +`node.class_name` through a chain of class-name lists; a class named in none of them falls through to the +default rule, which converts but gives types nobody chose. Adding a layer type means adding its branch there +too — a quiet omission, not a loud one. + +The user writes the name in pascal case in the config (`RecurrentReuseFactor`); it arrives as snake case +(`recurrent_reuse_factor`) in `layer.attributes`, and templates read it from there by the same name. + +## Worked example in the tree: the distributed arithmetic strategy + +A strategy implemented end to end as passes, useful as a template to copy. The machinery it uses is core to +every backend; the vocabulary it manipulates — `strategy`, `reuse_factor`, a `kernel` typedef chosen by a +config template — is a Vivado-family convention. Quartus and oneAPI have no `strategy` attribute at all, and +a backend under development may share none of these names. Passes remain the right tool there; only the +attribute names change. + +- `backends/vivado/passes/distributed_arithmetic.py` holds the whole feature: `DistributedArithmeticCodegen` + generates a per-layer C++ kernel from the weight values, `FuseQuantizerIntoDALayers` folds a neighbouring + operation in, and three template classes emit the layer's code. +- Ordering is the interesting part. The codegen pass is registered in the `optimize` flow **after** + `infer_precision_types`, because it needs final types to generate code from. The template passes are + registered in `specific_types`, after `transform_types` and `set_pipeline_style`. +- `VivadoBackend.init_dense` has a `distributed_arithmetic` branch that sets the attribute and raises when + the strategy is combined with a reuse factor it cannot support — validation belongs at the point the + attribute is set, not in the generated code. +- `DenseConfigTemplate.match` returns `False` for this strategy, so the common Dense template steps aside and + the strategy's own template runs instead. This is how a strategy replaces generated code wholesale. +- `backends/vitis/passes/feature_check.py` shows the shape of validation passes: match the unsupported + combination, then either warn and correct the attribute or raise. + +`backends/vivado/passes/unrolled_codegen.py` with the `resource_unrolled` strategy is a second example of the +same pattern at smaller scale. + +## Anti-checklist + +- Do not put model logic in the writer. If the writer needs to branch on a layer property, add a pass that + sets an attribute and let the writer copy it. +- Do not read `node.get_output_variable().type` for a final bit width in a pass that runs before precision + inference. +- Do not assume a pass in another backend's directory is available to you. Cross-backend names resolve, but + they create a dependency on that backend's flow; copy the pass into your own `passes/` instead. +- Do not return `True` from a pass that only sets attributes. +- Do not add an expected attribute without a default. +- After adding a pass, confirm it actually ran (print inside `transform`, or check the applied set) before + concluding that the effect you expected is impossible. diff --git a/.agents/precision-and-debugging.md b/.agents/precision-and-debugging.md new file mode 100644 index 0000000000..59c20290ea --- /dev/null +++ b/.agents/precision-and-debugging.md @@ -0,0 +1,76 @@ +--- +name: precision-and-debugging +description: >- + Choose and debug fixed-point precision in hls4ml, and inspect a generated project when the result is wrong. + Use when predict() or C simulation does not match the trained model, when setting ap_fixed types or + accumulator widths, or when you need to know which generated file to read. Applies to every backend. +globs: + - "hls4ml/model/types.py" + - "hls4ml/model/optimizer/passes/infer_precision.py" +--- + +# Precision, numerics, and reading a generated project + +Paths are relative to the package directory `hls4ml/hls4ml/`. For the kernels these types flow through, see +[kernels](kernels.md). + +## Precision and numerics + +- `FixedPrecisionType` (`model/types.py`) defaults to `width=16, integer=6, signed=True`, rounding + `TRN` (truncate) and saturation `WRAP`. **Overflow wraps silently**, which turns a slightly-too-small + integer width into a large sign-flipped error rather than a clipped value. When a layer's output is wildly + wrong but neighbouring layers look fine, suspect the integer width first. +- The accumulator is a separate type. `Layer._set_accum_t` fills `accum_t` from the config; a wide-enough + accumulator matters more than a wide output type, because every partial sum passes through it. +- Types left unset arrive as `UnspecifiedPrecisionType` and are filled by + `model/optimizer/passes/infer_precision.py`, which has per-layer-class rules. Automatic inference is + layer-local: it cannot know the data range, so it is conservative for weights and bias but not a substitute + for measuring. +- **Mixed precision is a first-class case.** Each inter-layer connection carries the *producer's* output + type, not a single model-wide type. A kernel that assumes one type for input, weights, accumulator and + output will fail as soon as layers differ; template on all four. +- Expected accuracy: for `ap_fixed<16,6>` on a small dense network, the maximum absolute difference against + the float reference lands at a few times `1e-3`. Errors at that scale are quantization and are not a bug. + Errors of order 0.1 or larger are a real defect: wrong weight layout, overflow, or an uninitialized + accumulator. +- Tools: `hls4ml.model.profiling.numerical(model=keras_model, hls_model=hls_model, X=X)` plots weight and + activation ranges against the chosen types; `compare(keras_model, hls_model, X)` shows per-layer + differences. For a layer-by-layer dump, set `Trace: True` in the layer config and call + `hls_model.trace(X)`, which recompiles with tracing and returns the intermediate tensors. + +## Reading a generated project + +File names below are the Vivado-family writer's. Other writers differ — Quartus and oneAPI put the headers +under `firmware/`, ship a Makefile or CMakeLists instead of tcl scripts, and Quartus writes separate parallel +and stream testbenches. The working method underneath the table holds regardless. + +| File | What it tells you | +| --- | --- | +| `firmware/myproject.cpp` | the real dataflow: stream declarations, pragmas, the order of layer calls | +| `firmware/parameters.h` | every config struct — the fastest check that an attribute reached C++ | +| `firmware/defines.h` | the concrete `ap_fixed<>` widths for each layer and variable | +| `firmware/weights/*.h` | the actual constants, in the layout the kernel receives | +| `myproject_bridge.cpp` | what `compile()` builds and `predict()` calls | +| `hls4ml_config.yml` | the fully resolved configuration, after all defaults and overrides | +| `vitis_hls.log`, `*_csynth.rpt` | scheduling, initiation interval and timing messages | + +Working method: + +1. **Check correctness before anything else.** `hls_model.compile()` then `hls_model.predict(X)` against the + float model — see [**running hls4ml**](running-hls4ml.md). C simulation is fast; do not synthesize a wrong design. +2. **Diff two generated projects** when comparing implementations. Generate one project per variant and + `diff -r` the `firmware/` directories. Everything except the intended difference should be identical; this + is how you prove a comparison is fair, and it repeatedly finds unintended differences. +3. **Use a fresh `output_dir`, or delete it first.** hls4ml does not clean the output directory on + re-conversion, so files from a previous run persist and you may be reading stale generated code. +4. **Read the logs, not only the parsed report.** A design that meets its latency target in C synthesis can + still fail timing in logic synthesis, and initiation-interval violations appear only in the log text. +5. **Which number to trust:** C synthesis is an estimate and is directional only; co-simulation gives the + true cycle count; logic synthesis gives the true resource use. Do not report C synthesis numbers as + final. See the [toolchain access](toolchain-access.md) and [evaluating implementations](evaluating-implementations.md) skills. + +## Anti-checklist + +- Do not treat a large numerical error as a precision problem before checking the weight layout, and do not + treat a small one as a bug. +- Do not conclude anything from a project directory you did not regenerate cleanly. diff --git a/.agents/reporting-issues.md b/.agents/reporting-issues.md new file mode 100644 index 0000000000..9a467db612 --- /dev/null +++ b/.agents/reporting-issues.md @@ -0,0 +1,118 @@ +--- +name: reporting-issues +description: >- + File a bug report, performance issue, feature request or RFC against hls4ml that a maintainer can act on. + Use when something appears broken, slower than expected, or missing, before opening anything on the issue + tracker. Covers deciding whether there is a bug at all, identifying which stage failed, reducing to a + minimal reproducer, and what each issue form actually needs. +globs: + - ".github/ISSUE_TEMPLATE/**" +--- + +# Reporting a problem in hls4ml + +The counterpart to [contributing changes](contributing-changes.md): that one covers submitting a fix, this +one covers reporting the problem. An issue is a request for someone else's time, so the work of narrowing it +down belongs to whoever files it. + +## Before filing anything + +1. **Reproduce on current `main`.** Bugs get fixed; the one you found may already be gone. Note the commit + you tested. +2. **Check it is not a configuration mistake.** Wrong precision, a reuse factor the layer cannot use, a + strategy the backend does not support, an `io_type` the layer family does not implement — these produce + confusing results and are not bugs. See [running hls4ml](running-hls4ml.md) for the arguments that decide + behaviour. +3. **Separate unsupported from broken.** Backends differ in what they implement. A layer that has no kernel + for a given backend or `io_type` is a missing feature, not a defect; file it as a feature request and say + which backend. +4. **Search the tracker.** Add to an existing issue rather than opening a second one. +5. **If you can fix it in a small, obvious change, send the pull request instead.** The contributing + guidelines ask contributors to spend effort on real improvements rather than on issue volume. + +Do not file speculative or theoretical problems — something that could go wrong in principle, or that a +reading of the code suggests, but that you have not observed. If you cannot reproduce it, you do not yet have +a report. + +## Identify which stage failed + +hls4ml is a pipeline, and the stage that fails determines what the report needs. Establish this before +writing anything. + +| Stage | Symptom | What the report needs | +| --- | --- | --- | +| Parsing | conversion fails on an unknown layer or attribute | frontend and its version, the layer, the model definition | +| Graph and passes | conversion succeeds but the graph is wrong; an exception from a pass | the failing pass name from the traceback, the config used | +| Writing | conversion succeeds, generated code is missing or malformed | the generated file and the part of it that is wrong | +| C simulation | `predict()` disagrees with the reference model | both outputs, the precision, and the tolerance you expected | +| Synthesis | the build fails, or the design does not meet timing | tool and version, the error from the log, the part and clock | + +A traceback usually names the stage directly. Include the **whole** traceback, not the last line. + +## Reduce to a minimal reproducer + +A report that arrives with a small script gets fixed; one that arrives with a research model usually does +not. Shrink it: + +- One layer if possible, with the smallest shapes that still fail, and a fixed random seed. +- Replace loaded weights with generated ones unless the values matter — if they do, say so, because that is + itself a clue. +- Put the model definition inline in the report rather than attaching a file. + +While shrinking, note which axes matter, because that is most of the diagnosis: + +- Does it depend on `io_type`? On the backend? On the strategy or reuse factor? +- Does it survive at a much wider precision such as `ap_fixed<32,16>`? If it disappears, the problem is + numerical rather than structural. + +## Telling quantization from a bug + +A fixed-point result never matches the float reference exactly. At `ap_fixed<16,6>` a small dense network +lands within a few times `1e-3`, and differences at that scale are quantization, not defects. Errors of order +`0.1` or larger indicate something real — overflow from too few integer bits, a wrong weight layout, an +uninitialized accumulator. Say which of the two you believe it is and why. See +[precision and debugging](precision-and-debugging.md). + +## Performance and resource issues + +These need numbers, and numbers need provenance. Say which stage produced them: C synthesis reports +estimates, co-simulation gives true latency, logic synthesis gives true resource usage. Quoting an estimate +as a measurement wastes a maintainer's time. + +Include the part, the clock period, the tool version, the model shape, the precision, the strategy and the +reuse factor, plus what you expected and where that expectation comes from. A report that some model is +"slow" without a configuration cannot be acted on. See +[evaluating implementations](evaluating-implementations.md) for producing comparable numbers. + +## Feature requests and RFCs + +A **feature request** describes a capability you need: the problem, why it matters for real users, and the +user-facing surface it would add. A **plan or RFC** proposes a design: motivation, mechanism, the components +affected, what happens on backends that cannot support it, and the migration for existing users. If your +proposal changes the intermediate representation, the optimizer machinery, or behaviour that existing +configurations depend on, it is an RFC and the guidelines ask for that discussion before implementation. + +## What not to put in an issue + +- Whole log files. Excerpt the error and the lines around it. +- Whole generated projects. Name the file and quote the part that matters. +- Screenshots of text. +- Guesses presented as findings. If you suspect a cause, mark it as a suspicion. + +## For agents specifically + +- **Never fabricate a traceback, an error message, a log line or a number.** Everything quoted must come from + a run that actually happened. If you have not run it, say so. +- If reproduction failed, report that outcome rather than filing an issue that implies it succeeded. +- Fill the issue form's fields; each one exists because reports kept arriving without it. +- Tick the AI assistance box on the form, and let the human check the content before it is posted. +- One issue per problem, and no duplicates — search first. + +## Anti-checklist + +- Do not open an issue you have not reproduced on current `main`. +- Do not report a configuration error as a bug without first checking the arguments you passed. +- Do not report a quantization difference as incorrect output. +- Do not quote C synthesis estimates as measured latency or resource usage. +- Do not attach a large model when a ten-line one shows the same failure. +- Do not file an issue for something a small pull request would fix. diff --git a/.agents/running-hls4ml.md b/.agents/running-hls4ml.md new file mode 100644 index 0000000000..d44eeebd87 --- /dev/null +++ b/.agents/running-hls4ml.md @@ -0,0 +1,109 @@ +--- +name: running-hls4ml +description: >- + Convert, compile and run an hls4ml model — the conversion API, the arguments that decide the outcome, and + the traps that make it fail. Use whenever you need to turn a Keras/PyTorch/ONNX model into an hls4ml + project, run predict() or trace(), check that a change works, or when `import hls4ml` behaves oddly. Read + this before any task that needs a model converted, because verifying anything else starts here. +globs: + - "hls4ml/converters/**" + - "hls4ml/utils/config.py" +--- + +# Running hls4ml + +## Environment + +hls4ml needs Python 3.10 or newer. Install it in editable mode (`pip install -e .`) when working on the tree, +so edits take effect without reinstalling. Frontend and tooling dependencies are optional extras, declared in +`pyproject.toml`: `keras-v3`, `onnx`, `profiling`, `da`, `hgq`, and others. Only the frontend you use needs to +be installed. + +For Keras, set `KERAS_BACKEND` (`tensorflow`, `torch` or `jax`) before importing, in the environment or at the +top of the script. + +Your site's own setup — where the interpreter lives, which extras are installed, how the HLS toolchain is +reached — belongs in a local file; see `local-setup.template.md` for a skeleton, and +[**toolchain access**](toolchain-access.md) for what a build needs on `PATH`. + +**Do not run Python from a directory that contains the `hls4ml` checkout directory.** It shadows the installed +package: `import hls4ml` then succeeds but yields an empty namespace package with `__file__` set to `None` and +no `converters` attribute. Run from anywhere else. If an attribute that obviously exists is missing, check the +working directory before anything else. + +## The recipe + +This runs as written: + +```python +import os +os.environ['KERAS_BACKEND'] = 'tensorflow' +import numpy as np, hls4ml +from tensorflow import keras + +model = keras.Sequential([keras.layers.Input((8,)), + keras.layers.Dense(6, activation='relu'), + keras.layers.Dense(3)]) + +cfg = hls4ml.utils.config_from_keras_model( + model, granularity='name', backend='Vitis', default_precision='fixed<16,6>') + +hmodel = hls4ml.converters.convert_from_keras_model( + model, hls_config=cfg, backend='Vitis', io_type='io_stream', output_dir='/tmp/prj') + +hmodel.compile() +X = np.random.rand(20, 8).astype('float32') +print(np.max(np.abs(hmodel.predict(X) - model.predict(X, verbose=0)))) +``` + +`compile()` writes the project and builds the shared library that `predict()` calls; it does not run the +vendor toolchain. `build()` does that, and is a separate, much slower step. + +## Arguments that decide the outcome + +- **`granularity`** in `config_from_keras_model`: `'model'` (one set of keys for everything, the default), + `'type'` (per layer class), `'name'` (per layer instance). Use `'name'` whenever a task involves setting + anything per layer; the other two silently give you no place to put it. +- **`backend`** defaults to `'Vivado'` in `convert_from_keras_model`, so it must be passed explicitly for + anything else, and passed to `config_from_keras_model` as well — the config helper asks the backend which + attributes are configurable, so a config built without it lacks backend-specific keys. +- **`io_type`** defaults to `'io_parallel'`. `'io_stream'` is a different code path in most layer families, + not a variation on the same one. +- **`part`**, **`clock_period`**, **`clock_uncertainty`** fall back to backend defaults when not given + (Vitis: `xcvu13p-flga2577-2-e`, 5 ns, 27%). Any comparison must fix them explicitly. +- **`default_precision`** accepts `'fixed<16,6>'` or a backend-specific spelling; `'auto'` is not allowed as + the default, but individual layer types may be set to `'auto'` to be inferred. +- Registered backends: `vivado`, `vivadoaccelerator`, `vitis`, `quartus`, `catapult`, `symbolicexpression`, + `oneapi`, `libero`. `hls4ml.backends.get_available_backends()` lists them. + +## What the numbers should look like + +For a small untrained dense network at `fixed<16,6>`, the maximum absolute difference against the Keras +prediction lands at a few times `1e-3`. That is quantization, not a bug. An error of order `0.1` or larger +means something real: overflow from too few integer bits, a wrong weight layout, or an uninitialized +accumulator. See [**kernels**](kernels.md) for the precision rules. + +## Inspecting what happened + +- The generated project is under `output_dir`; `firmware/parameters.h` is the fastest check that a config + value reached the generated code. +- **hls4ml does not clean `output_dir` on re-conversion.** Stale files from a previous run persist, and you + can spend a long time reading generated code that no longer corresponds to your change. Use a fresh + directory or delete it first. +- Per-layer intermediate values: set `Trace: True` in the layer config and call `hmodel.trace(X)`, which + recompiles with tracing and returns the intermediates alongside the prediction. +- Distributions against the chosen types: `hls4ml.model.profiling.numerical(model=keras_model, + hls_model=hmodel, X=X)`. + +## Common failures + +| Symptom | Cause | +| --- | --- | +| `hls4ml.__file__` is `None`, `converters` missing | running from a directory containing the checkout | +| `KeyError` on a layer class name during `config_from_keras_model` | the layer is parsed but not registered in `layer_map`; see [**frontends**](frontends.md) | +| "installation not found" from `build()` | the toolchain command is not on `PATH`; see [**toolchain access**](toolchain-access.md) | +| generated code does not match the change | stale `output_dir` | +| prediction is far from the reference | precision or layout, not the toolchain — check before synthesizing anything | + +Related: [**frontends**](frontends.md) for adding a layer the parser does not know, [**architecture map**](architecture-map.md) for +what happens between convert and write, [**evaluating implementations**](evaluating-implementations.md) for turning runs into a verdict. diff --git a/.agents/toolchain-access.md b/.agents/toolchain-access.md new file mode 100644 index 0000000000..a81fb2c230 --- /dev/null +++ b/.agents/toolchain-access.md @@ -0,0 +1,113 @@ +--- +name: toolchain-access +description: >- + Find, select and run the vendor toolchain an hls4ml backend needs (Vitis, Vivado, Quartus, oneAPI and + others), including when the tool is not installed on the host. Use whenever you need to synthesize or build + an hls4ml project, choose a tool version, launch a long build, or when a build fails with "installation not + found". Covers what each backend looks for, where a toolchain can come from, writing wrappers, version + selection, and which reported numbers to trust. +globs: + - "hls4ml/backends/*/*_backend.py" +--- + +# Getting an HLS toolchain and running a build + +## What hls4ml actually requires + +Every backend's `build()` looks for one command on `PATH` and runs it in the generated project directory. +Nothing more is required: satisfy the name on `PATH` and the backend is happy, whether the command is a real +installation, a module, or a wrapper script. + +| Backend | Command it looks for | What `build()` runs | +| --- | --- | --- | +| Vitis | `vitis-run` | `vitis-run --tcl build_prj.tcl --mode hls` | +| Vivado | `vivado_hls` | `vivado_hls -f build_prj.tcl "reset=… csim=… …"` | +| Quartus | `quartus_sh` | `make -fpga` in the project directory | +| oneAPI | the compiler toolchain used by the generated build files | `make ` in the build directory | + +Check the backend's `build()` in `hls4ml/backends//_backend.py` rather than assuming; the +command and its arguments change between releases. When the command is missing, the backend raises an +exception naming it — that message is the fastest diagnosis of a toolchain problem. + +## Where a toolchain can come from + +Vendor HLS tools are rarely on `PATH` by default. In order of how often they work: + +1. **A wrapper script your site already provides.** Check the local setup notes for the machine you are on; + many groups keep wrappers in a directory that only needs to be added to `PATH`. +2. **A local installation**, reached through a module system or by sourcing the vendor's settings script. +3. **A container image.** Vendor tools are commonly distributed as container images, sometimes as unpacked + directories on a shared filesystem. Which images exist is site-specific — list what is available rather + than assuming a path from documentation written elsewhere. +4. **Your own container image**, if nothing above provides the tool or the version you need. + +Record whichever applies in your site's local setup notes; see `local-setup.template.md`. + +## Writing a wrapper + +A wrapper is a script named exactly what the backend looks for, placed on `PATH`. For a container image: + +```bash +#!/usr/bin/env bash +exec exec --home "$HOME" -B \ + "$@" +``` + +Three details matter: + +- **Bind the filesystems your project lives on.** Paths not bound are invisible inside the container. + Runtimes usually bind `/tmp` and the current directory automatically; anything else has to be requested. +- **Give each run its own `HOME`.** Xilinx tools keep state in `$HOME/.Xilinx`. When several runs share it + they interfere and fail, either unable to load a Tcl package or crashing during logic synthesis. +- **Keep the wrapper's filename equal to the command name**, so both hls4ml's `build()` and generated build +- **Keep the wrapper's filename equal to the command name**, so both hls4ml's `build()` and generated build + scripts resolve it. + +Selecting a different version means pointing a wrapper at a different version directory. Keeping one wrapper +per version, named distinctly, and a plain-named wrapper for the default is a workable arrangement. + +## Choosing a version deliberately + +Results depend on the tool version: scheduling, resource estimates, achieved clock frequency and even +whether a design compiles all change between releases. So: + +- Use the **same version across every run of a comparison**, and say which one in any report of results. +- When reproducing an earlier result, reproduce its version too. +- When trying a newer version, treat it as a separate axis, not as a free upgrade — rerun the baseline. + +## Running a build + +Either call `model.build(...)` from Python, which runs the command in the table above, or run the same +command by hand in the generated project directory. Drive the tool through its scripted entry point rather +than a graphical session. + +Long builds take minutes to hours, so run them in a way that survives the calling process rather than +blocking on them, and avoid stacking extra shell layers around the launch — the intermediate process tends to +be orphaned and the build lost. Run one build per process, and never let two processes append to the same +results file, because interleaved writes corrupt it. Give each run its own output file. + +## Working directory, environment, storage + +- **Run from the generated project directory**, and never from a directory containing the hls4ml checkout — + it shadows the installed package. See [**running hls4ml**](running-hls4ml.md). +- Put build scratch on fast node-local storage rather than a shared or network filesystem, and copy the small + result files back afterwards. HLS builds write continuously, and write latency on shared storage is often + what limits them. + +## Which number to trust + +Four levels, in order; a design passes only when it clears all of them: + +1. **C simulation** — builds and runs, correct against the reference model. +2. **C synthesis** — early latency and resource *estimates*. Directional only; it under-reports multi-layer + io_stream latency. Never quote it as a final number. +3. **Co-simulation** — the true cycle count. Latency in any verdict comes from here. +4. **Logic synthesis** — the true LUT, FF, DSP and BRAM use, and the achieved clock. + +Read the logs, not only the parsed report: timing-closure failures, scheduling problems and initiation +interval violations appear as log text while the summary table still looks acceptable. Reports and logs are +under the solution directory of the generated project. Match part, clock period and tool version across any +two runs being compared. + +Related: [**evaluating implementations**](evaluating-implementations.md) for building a fair comparison on top of these runs, +[**kernels**](kernels.md) for reading the generated project itself. diff --git a/.gitignore b/.gitignore index 476728453b..f07c1975f6 100644 --- a/.gitignore +++ b/.gitignore @@ -16,3 +16,9 @@ hls4mlprj_* *.ipynb *.ipynb_checkpoints/ *.bak + +# Assistant-specific views generated from .agents/ by .agents/agent_adapters.py +/.claude/ +/.cursor/ +/.github/instructions/ +/agent-docs.md diff --git a/AGENTS.md b/AGENTS.md index 82eb19e9b5..3b4bf8830d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,6 +13,25 @@ frontend converter -> model graph -> optimizer passes -> code templates - hls4ml/converters/ hls4ml/model/ hls4ml/model/optimizer/, hls4ml/backends/*/passes/ hls4ml/writer/ ``` +## Where to read before you work + +Detailed notes live in [`.agents/`](.agents/README.md). Open the one that matches the task rather than +exploring the tree from scratch: + +| Task | Document | +| --- | --- | +| convert a model, run `predict()`, check a change works | [.agents/running-hls4ml.md](.agents/running-hls4ml.md) | +| anything inside the Python tree — what runs when, which file to open | [.agents/architecture-map.md](.agents/architecture-map.md) | +| add support for a layer or operator; a model fails to parse | [.agents/frontends.md](.agents/frontends.md) | +| change the graph, add a Strategy, an initializer or a config attribute | [.agents/optimizer-passes.md](.agents/optimizer-passes.md) | +| write or modify a C++ compute kernel | [.agents/kernels.md](.agents/kernels.md) | +| choose fixed-point types, or the numbers are wrong | [.agents/precision-and-debugging.md](.agents/precision-and-debugging.md) | +| claim one implementation is faster or smaller than another | [.agents/evaluating-implementations.md](.agents/evaluating-implementations.md) | +| stand up a backend for a new toolchain | [.agents/new-backend.md](.agents/new-backend.md) | +| synthesize, pick a tool version, or a build cannot find its tool | [.agents/toolchain-access.md](.agents/toolchain-access.md) | +| shape a change, and before opening a pull request | [.agents/contributing-changes.md](.agents/contributing-changes.md) | +| report a bug or a performance problem, or propose a feature | [.agents/reporting-issues.md](.agents/reporting-issues.md) | + ## Ground rules **Verify, do not assume.** hls4ml has several backends and they do not share conventions. A pragma, config diff --git a/test/pytest/test_agent_docs.py b/test/pytest/test_agent_docs.py new file mode 100644 index 0000000000..21dd7d2704 --- /dev/null +++ b/test/pytest/test_agent_docs.py @@ -0,0 +1,94 @@ +"""Checks that the agent documentation in .agents/ still matches the tree. + +These documents cite files, classes and functions. The cheapest way for them to become misleading is for one +of those to be renamed, so this test extracts every repository path they mention and asserts it exists. + +Paths are written in several styles: relative to the package directory (``backends/vivado/passes``), relative +to the repository root (``test/pytest``), or as a bare filename (``nnet_dense.h``). A token with a separator +has to match the tail of a real path; a bare filename has to match some file's name. Files belonging to a +*generated* project rather than to the repository are listed in GENERATED and skipped. +""" + +import re +from pathlib import Path + +import pytest + +repo_root = Path(__file__).parent.parent.parent +docs_dir = repo_root / '.agents' + +CHECKED_SUFFIXES = ('py', 'h', 'md', 'rst', 'tcl', 'yml', 'yaml', 'toml', 'in', 'cfg') + +# Names that appear in the documents but belong to a generated hls4ml project, not to this repository. +GENERATED = { + 'myproject.cpp', + 'myproject.h', + 'myproject_bridge.cpp', + 'myproject_test.cpp', + 'parameters.h', + 'defines.h', + 'hls4ml_config.yml', + 'build_prj.tcl', + 'build_opt.tcl', + 'project.tcl', + 'vitis_hls.log', +} + +path_like = re.compile(r'`([A-Za-z0-9_./*<>-]+\.(?:' + '|'.join(CHECKED_SUFFIXES) + r'))`') + + +def _repo_index(): + """All tracked-looking files in the repository, as a set of paths and a set of names.""" + paths, names = set(), set() + for path in repo_root.rglob('*'): + if not path.is_file() or '.git' in path.parts: + continue + relative = path.relative_to(repo_root) + paths.add(relative.as_posix()) + names.add(path.name) + return paths, names + + +REPO_PATHS, REPO_NAMES = _repo_index() +doc_files = sorted(docs_dir.glob('*.md')) + + +def _is_known(token: str) -> bool: + if token in GENERATED or Path(token).name in GENERATED: + return True + if token.startswith('firmware/') or '<' in token or '*' in token: + return True # generated output, or a placeholder such as _backend.py + if '/' in token: + return any(path == token or path.endswith('/' + token) for path in REPO_PATHS) + return token in REPO_NAMES + + +@pytest.mark.parametrize('doc', doc_files, ids=lambda p: p.name) +def test_referenced_paths_exist(doc): + """Every repository file named in a document should still exist.""" + missing = sorted({token for token in path_like.findall(doc.read_text()) if not _is_known(token)}) + assert not missing, f'{doc.name} refers to paths that no longer exist: {missing}' + + +@pytest.mark.parametrize('doc', doc_files, ids=lambda p: p.name) +def test_front_matter_is_well_formed(doc): + """Each document carries the metadata the adapter script needs.""" + if doc.name == 'README.md': + pytest.skip('the index carries no front matter') + match = re.match(r'^---\n(.*?)\n---\n', doc.read_text(), re.S) + assert match, f'{doc.name} has no front matter block' + front = match.group(1) + assert re.search(r'^name:\s*\S+', front, re.M), f'{doc.name} front matter has no name' + assert re.search(r'^description:\s*>-', front, re.M), f'{doc.name} front matter has no description' + assert re.search(r'^globs:', front, re.M), f'{doc.name} front matter has no globs' + + +@pytest.mark.parametrize('doc', doc_files, ids=lambda p: p.name) +def test_relative_links_resolve(doc): + """Links between the documents should point at files that exist.""" + broken = [ + target + for target in re.findall(r'\]\(([^)#][^)]*)\)', doc.read_text()) + if not target.startswith(('http://', 'https://', 'mailto:')) and not (doc.parent / target).resolve().exists() + ] + assert not broken, f'{doc.name} has broken relative links: {sorted(set(broken))}' From 64f6133867478bd55a786d297fd087d617e7047f Mon Sep 17 00:00:00 2001 From: Vladimir Loncar Date: Sun, 16 Aug 2026 17:56:07 +0200 Subject: [PATCH 4/4] Exclude .agents from check-manifest, document the manual-stage hooks --- .agents/contributing-changes.md | 15 +++++++++++++++ AGENTS.md | 4 +++- pyproject.toml | 1 + 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/.agents/contributing-changes.md b/.agents/contributing-changes.md index 12300e6996..00f8c3c873 100644 --- a/.agents/contributing-changes.md +++ b/.agents/contributing-changes.md @@ -53,6 +53,21 @@ pre-commit run --files 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: + +``` +pre-commit run --hook-stage manual --all-files +``` + +When `check-manifest` reports files "missing from sdist", decide which is right: content that ships with the +package belongs in `MANIFEST.in`, while documentation and repository furniture belong in the +`[tool.check-manifest] ignore` list in `pyproject.toml`, where `docs/**` and `.agents/**` already are. +Note that this hook needs the submodules checked out to give a meaningful answer. + ## Tests `CONTRIBUTING.md` requires a unit test under `test/` for new functionality, and for a bug fix a test that diff --git a/AGENTS.md b/AGENTS.md index 3b4bf8830d..7e7f58bf4d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -69,7 +69,9 @@ the person opening the pull request, and tell them what remains to be confirmed. ## Before proposing a change - Run `pre-commit run --files ` and commit what it changes. It formats Python and C++ and will - reject the pull request otherwise. + reject the pull request otherwise. If you added, moved or renamed files, also run + `pre-commit run --hook-stage manual --all-files`, which is what CI runs and which catches packaging + problems the default hooks do not. - Add or update a test under `test/pytest/`. For a bug fix, the test should fail without the fix. - Check the whole diff yourself, hunk by hunk, before presenting it. Remove debug output, commented-out code and stray changes. diff --git a/pyproject.toml b/pyproject.toml index e5d69b51fb..4549c00bb1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -123,6 +123,7 @@ lint.unfixable = [] [tool.check-manifest] ignore = [ + ".agents/**", ".github/**", ".pre-commit-config.yaml", "docs/**",