Skip to content

doc update for hgq/da #1359

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

calad0i
Copy link
Contributor

@calad0i calad0i commented Aug 7, 2025

Description

Adding & changing doc for HGQ2 and da4ml

If #1338 will be merged the precision propagation part will be updated accordingly

Type of change

  • Documentation update

Tests

N/A

Checklist

  • all

Copy link
Contributor

@vloncar vloncar left a comment

Choose a reason for hiding this comment

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

I think minor improvements to the docs could still be made

@@ -140,7 +140,8 @@ In this case, the HLS code is valid for both the Vivado and Quartus backends.
.. code-block:: Python

# Register the converter for custom Keras layer
hls4ml.converters.register_keras_layer_handler('KReverse', parse_reverse_layer)
hls4ml.converters.register_keras_v2_layer_handler('KReverse', parse_reverse_layer)
Copy link
Contributor

Choose a reason for hiding this comment

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

======================================

.. note::
HGQ2 is the successor of the original `HGQ <./hgq1.html>`__. framework, which was built on Keras v2. HGQ2 built on top of Keras v3, leveraging its new features and improvements.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe also add why people should switch, not just cuz it is Keras v3


.. code-block:: Python
Key Features
-----------
Copy link
Contributor

Choose a reason for hiding this comment

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

Lacks a -

@@ -0,0 +1,49 @@
===================================
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we put both of these under a subsection of "advanced". Other options is to introduce new section called "Concepts" and put the subsection there. And if the advice for new code is HGQ2, maybe add a note here and title this one to (deprecated)

.. [*] While quantized, the original model will still operate on float-point values, so there is a chance that the outputs will not be exactly the same due to float rounding errors in the original model.

.. note::
Unlike the automatic precision inference, it is strongly recommended to **not** use the ``config_from_*`` functions to set the precisions in the model.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is just confusing now. Why can't we say in HGQ docs that this is the bit-exact flow and users should not invoke the config_from_.... We can even make it fail if it encounters HGQ layers. And then merge this section with the HGQ


* **ReuseFactor**\ : in the case that you are pipelining, this defines the pipeline interval or initiation interval
* **ParallelizationFactor**\ : The number of output "pixels" to compute in parallel in convolutional layers. Increasing this parameter results in significant increase in resources required on the FPGA.
* **Strategy**\ : Optimization strategy on FPGA, either "Latency", "Resource" or "Unrolled". If none is supplied then hl4ml uses "Latency" as default. Note that a reuse factor larger than 1 should be specified when using "resource" or "unrolled" strategy. An example of using larger reuse factor can be found `here. <https://github.com/fastmachinelearning/models/tree/master/keras/KERAS_dense>`__
* **Strategy**\ : Optimization strategy on FPGA, either "Latency", "Resource", "distributed_arithmetic" (or "da"), or "Unrolled". If none is supplied then hl4ml uses "Latency" as default. Note that a reuse factor must be 1 if using "distributed_arithmetic", and must be larger than 1 when using "resource" or "unrolled" strategy.
Copy link
Contributor

Choose a reason for hiding this comment

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

I still feel da strategy should have been in InitCase and not snake_case. (or just use the word da so whatever the user puts lower() will match it.) Too late now...

* `HGQ <https://github.com/calad0i/HGQ>`_
The equivalent HGQ API is also supported. HGQ is not compatible with Keras v3. See `advanced/HGQ <../advanced/hgq.html>`__ for more information.
The equivalent HGQ API is also supported.
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a note that it is deprecated in favor of HGQ2?

Running C simulation from Python requires a C++11-compatible compiler. On Linux, a GCC C++ compiler ``g++`` is required. Any version from a recent
Linux should work. On MacOS, the *clang*-based ``g++`` is enough. For the oneAPI backend, one must have oneAPI installed, along with the FPGA compiler,
to run C/SYCL simulations.
Running C simulation from Python requires a C++11-compatible compiler. On Linux, a GCC C++ compiler ``g++`` is required. Any version from a recent Linux should work. On MacOS, the *clang*-based ``g++`` is enough. For the oneAPI backend, one must have oneAPI installed, along with the FPGA compiler, to run C/SYCL simulations.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add that it is not just any oneAPI it is only 2025.0?

+-----------------------+-----+-----+--------------+--------+--------+-----+
| Keras v3 | ✅ | ✅ | ✅ | N/A | ✅ | ❌ |
+-----------------------+-----+-----+--------------+--------+--------+-----+
| HGQ2 | ✅ | ✅ | N/A | N/A | ✅ | ✅ |
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you put N/A instead of ❌? keras v2 has einsum(dense), we just don't support it. brevitas supports rnn layers, so i think it is possible to get qonnx version too, we just don't support it. i think all of N/A should be ❌

* ``hls4ml`` supports Linux and requires python \>=3.10. hlsml does not require a specific Linux distribution version and we recommended to follow the requirements of the HLS tool you are using.
* Windows and macOS are not supported. Setting up ``hls4ml`` on these platforms, for example using the Windows Subsystem for Linux (WSL) should be possible, but we do not provide support for such use cases.

- Vivado HLS versions 2018.2 to 2020.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just say Vivado 2020.1? It sorta works with earlier versions, but it is always as "if you use this version, you're on your own".

Same for Vitis, we can say >=2022.2, versions after 2024.1 are tested less

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.

2 participants