-
Notifications
You must be signed in to change notification settings - Fork 51
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
Pynq-fud integration #1120
Pynq-fud integration #1120
Conversation
Previously, fud-pynq-script accepted a mapping and returned a string, now ittakes a path to an xclbin file and returns a dictionary conforming to json standards. The new execution.py retains all of the previous config settings while simplifying the kernel execution by running fud-pynq-script. execution.py is responsible for turning the script's output into a json file
…pynq-fud-integration
fud/fud/stages/xilinx/execution.py
Outdated
def import_libs(): | ||
"""Import optional libraries""" | ||
try: | ||
import pyopencl as cl # type: ignore | ||
|
||
self.cl = cl | ||
except ImportError: | ||
raise errors.RemoteLibsNotInstalled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasn't sure about this, should the pynq import from pyproject.toml
go here instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason this is inside a try
-catch
inside a function is because the fpga
dependencies are not installed by default. The program only throws an error if you try to run the xilinx
stage without installing the fpga
dependencies in the flit
configuration.
I would recommend moving the
from fud.stages.xilinx import fud_pynq_script
and using a self.pynq_script
field to access it instead of importing it on the top which will cause errors for default fud
installations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good! Address the comments about dependency to make sure that a fresh, default install of fud, when following these instructions does not break.
Once done, merge at your discretion
This PR can close #1114.
A new pynq-fud-script can take in arbitrary data, run a kernel on it, and output the relevant memories. Integrates into
fud e foo.xclbin --from xclbin --to fpga ...
commands.This replaces the old OpenCL way of running a kernel that fud used to use.
Unfortunately this does not address #1037 #872 #856.