Pointwise Conv1D with code generation for "Latency" strategy (update of #811) #881

jmduarte · 2023-10-08T05:18:06Z

Testing update of #811

hls4ml/backends/fpga/fpga_backend.py

hls4ml/backends/fpga/passes/codegen.py

hls4ml/backends/vivado/passes/convolution_templates.py

vloncar · 2023-10-11T17:26:10Z

hls4ml/backends/vivado/vivado_backend.py

-            # attrs.append(ConfigurableAttribute('conv_implementation', value_type=str, default='LineBuffer'))
-            attrs.append(ChoiceAttribute('conv_implementation', choices=['LineBuffer', 'Encoded'], default='LineBuffer'))
+            attrs.append(
+                ChoiceAttribute('conv_implementation', choices=['LineBuffer', 'Encoded', 'Pointwise'], default='LineBuffer')


So this basically means that I can put Pointwise as the implementation for layers that are not pointwise? that's not user-friendly because if it causes the pointwise hls functions to be used they will fail an assert, which is invisible when calling from python.

I agree this is bad, but I'm not sure the best way to fix it. Is it as simple as checking if a user tries to do this and exiting gracefully with a warning message?

Perhaps I'm forgetting other cases here, but wasn't this shown to have better performance than existing implementation? Therefore one could argue we don't really need a switch to choose algorithms we know are worse. I'd make this default and not hide it behind a (currently undocumented) switch.

hls4ml/templates/vivado/build_prj.tcl

hls4ml/templates/vivado/nnet_utils/nnet_code_gen.h

vloncar · 2023-10-11T17:30:48Z

hls4ml/templates/vivado/nnet_utils/nnet_common.h

@@ -24,6 +24,7 @@ namespace nnet {
 // Common type definitions
 enum io_type { io_parallel = 0, io_stream };
 enum strategy { latency, resource };
+enum class conv_implementation { linebuffer = 0, encoded = 1, pointwise = 2 };


Same question as before

vloncar · 2023-10-11T17:38:36Z

hls4ml/templates/vivado/nnet_utils/nnet_conv1d.h

+        if (CONFIG_T::implementation == conv_implementation::pointwise) {
+            // Use pointwise unrolled implementation
+            if (CONFIG_T::reuse_factor > 1) {
+                CONFIG_T::template pointwise_conv<data_T, res_T, CONFIG_T>::pointwise_conv(data, res, weights, biases);


So a model with a kernel > 1 but set conv_implementation to pointwise, the code will be generated and since it doesn't have assert it will proceed to do an incorrect computation? It's nonsense to us, but I fear that misunderstanding of the docs or just bugs in conversion scripts may trigger this

So there is an assert here:

hls4ml/hls4ml/templates/vivado/nnet_utils/nnet_conv1d_latency.h

Lines 88 to 92 in 1dd2603

void pointwise_conv_1d_latency_cl(data_T data[CONFIG_T::in_width * CONFIG_T::n_chan / CONFIG_T::reuse_factor],

res_T res[CONFIG_T::out_width * CONFIG_T::n_filt / CONFIG_T::reuse_factor],

typename CONFIG_T::weight_t weights[CONFIG_T::n_chan * CONFIG_T::n_filt],

typename CONFIG_T::bias_t biases[CONFIG_T::n_filt]) {

assert(CONFIG_T::filt_width == 1);

Where else should we add it?

vloncar · 2023-10-11T17:39:24Z

test/pytest/test_pointwiseconv.py

@@ -100,6 +105,7 @@ def test_pointwiseconv2d(chans, padds, strides, backend, io_type, strategy):
            kernel_initializer='normal',
            use_bias=False,
            data_format=chans,
+            name='pointwise2d',


general comment, not specific to this line: why don't we do this for 2d as well?

Yes, we can easily add 2d... working on it.

jmduarte and others added 14 commits June 21, 2023 07:23

merge

ea5c5a8

add pointwise

6849e0b

latency

0244b66

unroll

3ae7752

add hls unroll

23126b7

fix pragma from walkie

6aff9e9

[pre-commit.ci] auto fixes from pre-commit hooks

7f1c318

add test

69aecc6

pre-commit

4febced

pre-commit

56797e7

Merge branch 'main' into split_pointwise_conv_by_rf_rebase_latest

fc8cf1f

Merge branch 'main' into split_pointwise_conv_by_rf_rebase_latest

48609c6

Merge branch 'main' into split_pointwise_conv_by_rf_rebase_latest

7e11eea

use code gen

a01080d

jmduarte added the please test Trigger testing by creating local PR branch label Oct 8, 2023

fix indent

30c5c70

jmduarte added please test Trigger testing by creating local PR branch and removed please test Trigger testing by creating local PR branch labels Oct 8, 2023

jmduarte mentioned this pull request Oct 9, 2023

Explicit pointwise Conv1D implementation for "Latency" strategy #811

Closed

8 tasks

update rf

a05bf69

jmduarte added please test Trigger testing by creating local PR branch and removed please test Trigger testing by creating local PR branch labels Oct 11, 2023

vloncar reviewed Oct 11, 2023

View reviewed changes

jmduarte added 2 commits October 11, 2023 19:58

address vlad comments part 1

445b2cd

default 4096

1dd2603

jmduarte added please test Trigger testing by creating local PR branch and removed please test Trigger testing by creating local PR branch labels Oct 12, 2023

only add pointwise function when optimizing

04997c2

jmduarte added please test Trigger testing by creating local PR branch and removed please test Trigger testing by creating local PR branch labels Oct 15, 2023

jmduarte changed the title ~~Testing update of #811~~ Pointwise Conv1D/2D with code generation for "Latency" strategy (update of #811) Oct 20, 2023

jmitrevs added this to the v1.0.0 milestone Oct 20, 2023

Merge branch 'main' into split_pointwise_conv_by_rf_codegen

8b1c44d

jmduarte added please test Trigger testing by creating local PR branch and removed please test Trigger testing by creating local PR branch labels Dec 19, 2023

jmduarte changed the title ~~Pointwise Conv1D/2D with code generation for "Latency" strategy (update of #811)~~ Pointwise Conv1D with code generation for "Latency" strategy (update of #811) Apr 23, 2024

jmduarte added 3 commits May 28, 2024 15:15

Merge branch 'main' into split_pointwise_conv_by_rf_codegen

3d8912d

Merge branch 'main' into split_pointwise_conv_by_rf_codegen

8a48417

add vitis

a181d97

jmduarte added please test Trigger testing by creating local PR branch and removed please test Trigger testing by creating local PR branch labels Jun 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pointwise Conv1D with code generation for "Latency" strategy (update of #811) #881

Pointwise Conv1D with code generation for "Latency" strategy (update of #811) #881

jmduarte commented Oct 8, 2023

vloncar Oct 11, 2023

jmduarte Oct 15, 2023

vloncar Oct 15, 2023

vloncar Oct 11, 2023

vloncar Oct 11, 2023

jmduarte Oct 15, 2023

vloncar Oct 11, 2023

jmduarte Oct 15, 2023

	void pointwise_conv_1d_latency_cl(data_T data[CONFIG_T::in_width * CONFIG_T::n_chan / CONFIG_T::reuse_factor],
	res_T res[CONFIG_T::out_width * CONFIG_T::n_filt / CONFIG_T::reuse_factor],
	typename CONFIG_T::weight_t weights[CONFIG_T::n_chan * CONFIG_T::n_filt],
	typename CONFIG_T::bias_t biases[CONFIG_T::n_filt]) {
	assert(CONFIG_T::filt_width == 1);

Pointwise Conv1D with code generation for "Latency" strategy (update of #811) #881

Are you sure you want to change the base?

Pointwise Conv1D with code generation for "Latency" strategy (update of #811) #881

Conversation

jmduarte commented Oct 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment