Closed bo3z closed 1 year ago
pytest.activations is failing:
E AssertionError:
E Not equal to tolerance rtol=0.02, atol=0.02
E
E Mismatched elements: 8000 / 8000 (100%)
E Max absolute difference: 1.12238881
E Max relative difference: 8914.97600568
E x: array([[0.793945, 0.791992, 0.798828, ..., 0.804688, 0.791016, 0.799805],
E [0.791016, 0.802734, 0.804688, ..., 0.799805, 0.799805, 0.794922],
E [0.795898, 0.808594, 0.803711, ..., 0.793945, 0.796875, 0.801758],...
E y: array([[-0.227973, -0.279667, -0.045713, ..., 0.226889, -0.28958 ,
E 0.031885],
E [-0.292061, 0.154492, 0.214236, ..., 0.041079, -0.003215,...
test_activations.py:55: AssertionError
Can you see why?
pytest.activations is failing:
E AssertionError: E Not equal to tolerance rtol=0.02, atol=0.02 E E Mismatched elements: 8000 / 8000 (100%) E Max absolute difference: 1.12238881 E Max relative difference: 8914.97600568 E x: array([[0.793945, 0.791992, 0.798828, ..., 0.804688, 0.791016, 0.799805], E [0.791016, 0.802734, 0.804688, ..., 0.799805, 0.799805, 0.794922], E [0.795898, 0.808594, 0.803711, ..., 0.793945, 0.796875, 0.801758],... E y: array([[-0.227973, -0.279667, -0.045713, ..., 0.226889, -0.28958 , E 0.031885], E [-0.292061, 0.154492, 0.214236, ..., 0.041079, -0.003215,... test_activations.py:55: AssertionError
Can you see why?
This was addressed in a PR #655 that was already merged. It comes from the fact that the parallel Softsign was optimised in #585, by removing unnecessary values in the LUT but required changes in logic.
It generally looks good to me so I approved it. I sort of wanted to trigger the pytests again, but couldn't figure out how.
I can merge it later today unless someone wants to check more.
I need some more time to go through this.
@jmitrevs All the issues have been resolved. Do you want to take another pass at this or we merge it?
Using a slightly older branch, I noticed that in a project I created the using stream definition is in both defines.h and nnet_helpers.h. Is that still the case and needed? (I was hacking the definition in one and I got an error that the two definitions didn't match.
I removed the definitions from nnet_helpers.h
. All tests (python compile, make and quartus compile) pass.
The only issue remaining with this PR is that occasionally the padding routines don't work with a cryptic error from the compiler: Compiler Error: Multiple reflexive accesses from stream 'layer2_out' is not allowed
. This happens for ZeroPadding1D/2D and Conv1D/2D (with same padding) under certain scenarios. This still needs some understanding, potentially with help from Intel, so I wouldn't block the merge of this just because of that. @jmitrevs?
Just for completeness, this alternate unoptimized 1d padding implementation does not suffer the error:
template<class data_T, class res_T, typename CONFIG_T>
void zeropad1d_cl(stream<data_T> &data, stream<res_T> &res) {
res_T res_array[CONFIG_T::out_width];
ZeroOutputArray:
for (int i = 0; i < CONFIG_T::out_width; i++) {
for (int j = 0; j < CONFIG_T::n_chan; j++) {
res_array[i][j] = 0;
}
}
CopyMain:
for (int i = 0; i < CONFIG_T::in_width; i++) {
auto dataval = data.read();
for (int j = 0; j < CONFIG_T::n_chan; j++) {
res_array[i+CONFIG_T::pad_left][j] = dataval[j];
}
}
StreamOut:
for (int i = 0; i < CONFIG_T::out_width; i++) {
res.write(res_array[i]);
}
}
Nevertheless, why what we have fails is not clear to me. I'll leave some time for comments, but if no one objects, we can merge this weekend.
Description
Type of change
Tests
All of the existing tests were expanded to include tests for Quartus in
io_stream
. No new tests were written. A summary of the tests is given below.Synthesis results
Below are results obtained through full Quartus synthesis of Conv2D layers for a fixed input (32x32x3) when varying the number of filters and the reuse factors. Other layers were tested for correct synthesis.
Checklist