Skip to content

oneAPI backend update: report #1222

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

Merged
merged 17 commits into from
Apr 8, 2025
Merged

Conversation

enlupi
Copy link
Contributor

@enlupi enlupi commented Mar 11, 2025

Description

Added functionalities for parsing and printing the QoR report when using oneAPI backend.

Type of change

  • New feature (non-breaking change which adds functionality)

This PR introduces the possibility of parsing and printing the oneAPI report, akin to what it is already possible to do using Vivado backend. All the necessary functions are contained in the hls4ml/report/oneapi_report.py file.

Tests

Modified test/pytest/test_report.py to test parsing and printing of the report:

  • Added oneAPI backend test while manteining functionalities for Vivado
  • Restructured code so that the HLS model setup is executed before the test proper: the test is marked as failed only if errors happen during the parsing and printing of report
  • Restructured the test_report directory so that Vivado and oneAPI files are included in separated subdirectories
  • Output project directories are now temporary and deleted after test execution

Test Configuration:

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 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.

@jmitrevs jmitrevs added the please test Trigger testing by creating local PR branch label Mar 11, 2025
@jmitrevs jmitrevs added please test Trigger testing by creating local PR branch and removed please test Trigger testing by creating local PR branch labels Mar 12, 2025
@enlupi enlupi marked this pull request as ready for review March 20, 2025 13:41
@jmitrevs
Copy link
Contributor

The Vivado/Vitis backends are also integrated with the build command, so that it can return a dictionary of results. Do we want to also add such functionality?

@vloncar
Copy link
Contributor

vloncar commented Mar 20, 2025

Yes, all backends do this, so we should keep consistent. Should just be a line return parse_oneapi_report(outdir) at the end of OneAPIBackend.build(), right?

@enlupi
Copy link
Contributor Author

enlupi commented Mar 21, 2025

I added the change as suggested in the previous comments. Please let me know if further work is needed on this.

@jmitrevs jmitrevs added please test Trigger testing by creating local PR branch and removed please test Trigger testing by creating local PR branch labels Mar 26, 2025
@jmitrevs
Copy link
Contributor

Is this generally complete and ready for review?

@jmitrevs
Copy link
Contributor

This needs documentation so that we know how to use it.

@enlupi
Copy link
Contributor Author

enlupi commented Mar 27, 2025

Yes, this is generally complete, unless of course you have idea on how to improve it.

As for the documentation, would something simple like the one for the Quartus report work?

@jmitrevs
Copy link
Contributor

jmitrevs commented Apr 7, 2025

What is the user interface? How is the user supposed to print the report?

@enlupi
Copy link
Contributor Author

enlupi commented Apr 8, 2025

The user interface is the same as for the Vivado backend. With parse_vivado_report the user provides the hls project directory and gets as output a dictionary containing the relevant information. WIth print_oneapi_report the user provides the dictionary and it prints the report (as a nice html table if the code is run on a notebook, or just as a "string" table if run on the terminal)

@jmitrevs
Copy link
Contributor

jmitrevs commented Apr 8, 2025

The only interface we have ever "documented" is read_vivado_report in the tutorial. I don't think most people know how to use this. We definitely need to improve the documentation for all the backends, not just for this one. (People were wondering if read_vivado_report changed to read_vitis_report, for example.) And I still think after hls_model.build() we should be able to say hls_model.report() or something similar. But maybe all those improvements are outside of this scope of this PR. (We should also update the oneAPI branch of the tutorial using these utilities.)

@jmitrevs jmitrevs merged commit b1dfe3e into fastmachinelearning:main Apr 8, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
please test Trigger testing by creating local PR branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants