Skip to content

Rename shadowed loop variables in plot utility and writers - #1602

Closed
JanFSchulte wants to merge 1 commit into
mainfrom
fix/shadowed-loop-variables
Closed

JanFSchulte wants to merge 1 commit into
mainfrom
fix/shadowed-loop-variables

Conversation

@JanFSchulte

Copy link
Copy Markdown
Contributor

Description

The inner loops over layer functions in the Catapult, Coyote, Libero and Vivado writers reused the name of the outer template line variable. Rename it to func_line, and likewise rename the shadowing loop variable in model_to_dot.

Fixes the newly appearing flake8 errors B045.

Type of change

  • 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:

  • Catapult, Coyote, Libero and Vivado are technically affected, but no real change expected
  • 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

pre-commit passes now.

Breaking changes and migration

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): Claude Opus 5.5

Where it was used: Verified that the pre-commit findings weren't spurious and then fixed them

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

Required:

  • I have read the contributing guidelines.
  • 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


The inner loops over layer functions in the Catapult, Coyote, Libero and
Vivado writers reused the name of the outer template line variable. Rename
it to func_line, and likewise rename the shadowing loop variable in
model_to_dot.
@JanFSchulte

Copy link
Copy Markdown
Contributor Author

Ah, this is included in #1599 already.

@JanFSchulte JanFSchulte closed this Nov 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant