Conversation
…-backend (fastmachinelearning#1347) Merge branch 'init_interval_fix_zeropad_maxpooling' into coyote-accelerator-and-pooling
There was a problem hiding this comment.
A few misc comments based on trying to run the CoyoteAccelerator for a dummy model. Right now, i am stuck with a python import error:

which is puzzling because I do have jinja2 installed in my environment and the same import works fine in an interactive python session.
Also, can you fix the pre-commit issues?
| filedir = os.path.dirname(os.path.abspath(__file__)) | ||
| srcpath = os.path.join(filedir, '../contrib/Coyote/') | ||
| dstpath = f'{model.config.get_output_dir()}/Coyote' | ||
| copytree(srcpath, dstpath) |
There was a problem hiding this comment.
Do we want to use the dirs_exist_ok argument here? In the current version, this fails when running for the same project twice.
| ) | ||
|
|
||
| if not os.path.exists(f'{model.config.get_output_dir()}/build/{model.config.get_project_name()}_cyt_hw'): | ||
| os.mkdir(f'{model.config.get_output_dir()}/build/{model.config.get_project_name()}_cyt_hw') |
There was a problem hiding this comment.
I think this needs to use os.makedirs() because the build folder doesn't exist already.
vloncar
left a comment
There was a problem hiding this comment.
Looks good. I have mostly questions for better understanding and minor nitpicks that I don't feel are crucial, rather optional.
| if not os.path.exists(f'{model.config.get_output_dir()}/build/{model.config.get_project_name()}_cyt_hw'): | ||
| os.mkdir(f'{model.config.get_output_dir()}/build/{model.config.get_project_name()}_cyt_hw') | ||
| os.chdir(f'{model.config.get_output_dir()}/build/{model.config.get_project_name()}_cyt_hw') | ||
| os.system(cmake_cmd) |
There was a problem hiding this comment.
I would argue that new code (despite being inspired by existing code) should use subprocess instead of os.system and we gradually move towards phasing out os.system since it has limitations on tracking status.
| if len(X.shape) == 1: | ||
| X = np.array([X]) | ||
| if not (isinstance(X.dtype, float) or isinstance(X.dtype, np.float32)): | ||
| logging.warning('CoyoteOverlay only supports (for now) floating-point inputs; casting input data to float') |
There was a problem hiding this comment.
Have we completely switched to logging module or we still use warnings. Do these two play along nicely?
There was a problem hiding this comment.
I am not sure.
I see three instances in the code:
- In some cases the function warn(...) is used.
- In some other cases, a normal print is used, e.g., print('WARNING:...')
- Some code (mostly mine), uses logging.warning.
I've never had issues with logging.warning but happy to change as needed.
| #include "defines.h" | ||
| #include "host_libs.hpp" | ||
|
|
||
| #include <boost/program_options.hpp> |
There was a problem hiding this comment.
Does this require us to have Boost library installed on the host or that comes with Coyote?
There was a problem hiding this comment.
Yes, it's required, and, no it's not installed with Coyote.
One can argue that Boost shouldn't be needed for something as simple as CLI parsing, but Coyote relies internally on Boost for some other functionality (inter-process mutexes).
Currently, Coyote's compilation flow (i.e. CMake) will check if Boost is installed and throw an error if not. To me this sounds okay, as Boost is a fairly straightforward library to install.
| avg_latency += (time / 1e3); | ||
| avg_throughput += (batch_size / (time * 1e-9)); | ||
|
|
||
| // Functional correctness |
There was a problem hiding this comment.
Do you really need this? Is there a way to just run without checks, as in production?
There was a problem hiding this comment.
I can add a CLI argument, which is disabled by default?
|
|
||
| } | ||
|
|
||
| std::cout << "Batches processed: " << total_batches << std::endl; |
There was a problem hiding this comment.
Similar comment to the Python side with predict()
There was a problem hiding this comment.
I can add a CLI argument, verbose?
|
|
||
| filedir = os.path.dirname(os.path.abspath(__file__)) | ||
|
|
||
| f = open(os.path.join(filedir, '../templates/vivado/firmware/myproject.cpp')) |
There was a problem hiding this comment.
Imagine how cool it would be if we used pathlib.Path (which is imported) and resource management (with) like the other modern backends do... 😄
There was a problem hiding this comment.
This is several months old now, do you want to update it?
There was a problem hiding this comment.
Yes, once I open-source and merge the V80 code with upstream (hopefully next week), I can update the submodule.
|
Just confirming that installing |
|
That's good to now. At some point, I should update Coyote to make sure that jina isn't required system-wide. If it also works for inference, that's great. |
FIFO Optimization fix and add aclk_freq as parameter
Description
Generally, Coyote offers several advantages, when compared to some other shells, including:
The backend is briefly described in Section 9.7 of the paper: https://arxiv.org/pdf/2504.21538.
Type of change
Tests
This backend was compared agains a modified* version of the VivadoAccelerator backend: the backend was modified to run HLS synthesis with Vitis instead of Vivado (also using Vitis templates and optimizers), while the rest of the backend infrastructure (drivers, data movers remained the same since they also work in newer version of Vivado). Results are attached below - clearly indicating an advantage in Coyote, for two reasons (1) optimised data movement, bypassing card memory and (2) optimised host-side library (Python, C++).
In principle, the correct test would be to compare against VitisAccelerator (#991), but only after the io_parallel issues are resolved. However, the expectation is that the result will stay mostly the same, sine the underlying platform requires a data copy between host and card memory.
Will add some more results, also for io-stream CNN, and comparisons to VitisAccelerator.
Figure above: comparison of CoyoteAccelerator with modified Vivado Accelerator for the UNSW-NB15 dataset in io_parallel.
Checklist
pre-commiton the files I edited or added.