Skip to content
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

Quartus Extensions #628

Merged
merged 1 commit into from
Aug 12, 2022
Merged

Conversation

bo3z
Copy link
Contributor

@bo3z bo3z commented Aug 3, 2022

A# Description

📝 Support for custom Keras and HLS layers

  • Simple re-ordering of optimizers in the Quartus backend allows to register a new layer function/config template, alongside any optimizers linked to it.

Type of change

For a new feature or function, please create an issue first to discuss it
with us before submitting a pull request.

Note: Please delete options that are not relevant.

  • Bug fix (non-breaking change that fixes an issue) - This should have worked before in a similar manner way to Vivado, but was never tested. This PR simply adds a PyTest and fixed flow requirements to allow custom Keras layers to be introduced.

Tests

  • Expanded test/pytest/test_extensions.py to include Quartus (on top of Vivado)

Checklist

  • I have read the guidelines for contributing.
  • [] 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 added tests that prove my fix is effective or that my feature works.B

@@ -74,8 +97,6 @@ def format(self, node):
data_T input[CONFIG_T::n_in],
data_T reversed[CONFIG_T::n_in]
) {
#pragma HLS PIPELINE
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this line cause problems for Intel HLS compiler? What if we wrap it in #ifdef __SYNTHESIS__ and do similar for Intel compiler

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it won't (I think there is a flag for ignoring unknown pragmas), but PyTests do not run HLS synthesis anyway, so this line does not really do anything. Only reason I removed it is to make it "technically" correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

That entire file used to be an example of extensions API that also served as a test, so that block is an example of (Vivado) HLS code that users should write themselves. I don't see how distilling it to pure C++ code makes it "tecnhically" correct ;-) But it doesn't matter, since now it is a test that serves as a (slightly convoluted) example.

@vloncar vloncar merged commit 9f71389 into fastmachinelearning:main Aug 12, 2022
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