diff --git a/fpga/hls/spot_finder.cpp b/fpga/hls/spot_finder.cpp index ca517a97f..29069cc9f 100644 --- a/fpga/hls/spot_finder.cpp +++ b/fpga/hls/spot_finder.cpp @@ -2,9 +2,9 @@ #include "hls_jfjoch.h" -#define SUM_BITWIDTH 28 // 16 + 11 + 1 -#define SUM2_BITWIDTH 44 // 32 + 11 + 1 -#define VALID_BITWIDTH 11 // 11 +#define SUM_BITWIDTH 29 // 16 + 12 + 1 +#define SUM2_BITWIDTH 45 // 32 + 12 + 1 +#define VALID_BITWIDTH 12 // 12 #ifdef JFJOCH_HLS_NOSYNTH #include @@ -128,23 +128,31 @@ void spot_finder_prepare(hls::stream &data_in, } ap_uint<32> spot_finder_snr_threshold(ap_int<16> val[32], - ap_uint<8> snr_threshold, + ap_uint<16> snr_threshold_2, ap_int sum, ap_int sum2, ap_int valid_count) { #pragma HLS PIPELINE II=1 - if (snr_threshold == 0) + if (snr_threshold_2 == 0) return UINT32_MAX; - ap_int variance = valid_count * sum2 - sum * sum; - ap_int threshold = (variance * snr_threshold * valid_count) / 4; + ap_int variance = valid_count * sum2 - sum * sum; // This is variance * valid_count^2 + ap_int threshold = ((variance * snr_threshold_2 + 8) / (4*4)); // snr_threshold is in units of 0.25 ap_uint<32> ret = 0; for (int j = 0; j < 32; j++) { - ap_int in_minus_mean = val[j] * valid_count - sum; - if ((in_minus_mean * in_minus_mean * (valid_count - 1) > threshold) && - (in_minus_mean > 0)) + ap_int in_minus_mean = val[j] * valid_count - sum; // This is (pxl - mean) * valid_count + + // Aim is to compare pxl-mean with sqrt(variance) * threshold + // however this would require sqrt and divisions, so + // it is cheaper to compare ((pxl-mean) * valid_count)^2 with variance * valid_count^2 * threshold^2, + // but need to make sure that (pxl - mean) is positive + // Also assume that N ≈ (N-1) + + if ((in_minus_mean * in_minus_mean > threshold) && + (in_minus_mean > 0) && + (valid_count > 32 * 32 / 2)) // at least half of the pixels ret[j] = 1; else ret[j] = 0; @@ -188,6 +196,7 @@ void spot_finder_apply_threshold(hls::stream &data_in, while (!packet_in.user) { ap_int<16> count_threshold = in_count_threshold; ap_uint<8> snr_threshold = in_snr_threshold; + ap_uint<16> snr_threshold_2 = snr_threshold * snr_threshold; ap_uint<32> strong_pixel_count = 0; for (int i = 0; i < RAW_MODULE_SIZE * sizeof(uint16_t) / 64; i++) { #pragma HLS PIPELINE II=1 @@ -203,7 +212,7 @@ void spot_finder_apply_threshold(hls::stream &data_in, unpack32(packet_in.data, data_unpacked); ap_uint<32> strong_pixel = spot_finder_count_threshold(data_unpacked, count_threshold) & - spot_finder_snr_threshold(data_unpacked, snr_threshold, + spot_finder_snr_threshold(data_unpacked, snr_threshold_2, sum[i % 32], sum2[i % 32], valid[i % 32]); strong_pixel_out << ap_axiu<32,1,1,1>{.data = strong_pixel, .user = 0}; diff --git a/receiver/FPGAAcquisitionDevice.cpp b/receiver/FPGAAcquisitionDevice.cpp index dbe7be2d7..0a4f54f76 100644 --- a/receiver/FPGAAcquisitionDevice.cpp +++ b/receiver/FPGAAcquisitionDevice.cpp @@ -328,6 +328,6 @@ void FPGAAcquisitionDevice::SetSpotFinderParameters(int16_t count_threshold, dou logger->Warning("Trying to set SNR threshold too high: {}", snr_threshold ); snr_threshold = 64; } - SpotFinderParameters params{.count_threshold = count_threshold, .snr_threshold = to_fixed(snr_threshold, 4)}; + SpotFinderParameters params{.count_threshold = count_threshold, .snr_threshold = to_fixed(snr_threshold, 2)}; HW_SetSpotFinderParameters(params); } diff --git a/tests/FPGAIntegrationTest.cpp b/tests/FPGAIntegrationTest.cpp index 3f30dd969..b886f866a 100644 --- a/tests/FPGAIntegrationTest.cpp +++ b/tests/FPGAIntegrationTest.cpp @@ -1181,4 +1181,52 @@ TEST_CASE("HLS_C_Simulation_internal_packet_generator_spot_finder_count_threshol REQUIRE (spot_finder_result.strong_pixel[0] == (1<<0)); REQUIRE (spot_finder_result.strong_pixel[(123*1024 + 578) / 8] == (1<<2)); // 578 % 8 == 2 REQUIRE (spot_finder_result.strong_pixel[(121*1024 + 800) / 8] == (1<<0)); // 800 % 8 == 0 +} + +TEST_CASE("HLS_C_Simulation_internal_packet_generator_spot_finder_snr_threshold", "[FPGA][Full]") { + const uint16_t nmodules = 4; + + DiffractionExperiment x((DetectorGeometry(nmodules))); + + x.Mode(DetectorMode::Raw); + x.UseInternalPacketGenerator(true).ImagesPerTrigger(1).PedestalG0Frames(0); + + HLSSimulatedDevice test(0, 64); + + std::vector frame(RAW_MODULE_SIZE); + for (int i = 0; i < RAW_MODULE_SIZE; i++) { + frame[i] = ((i / RAW_MODULE_COLS) + (i % RAW_MODULE_COLS)) % 2; + } + // Mean = 0.5 + // Std. dev. = 0.5 + // Threshold = 10 * std. dev. - 6 is minimum count + + frame [ 0] = 8; + frame [123*1024 + 578] = 5; + frame [121*1024 + 800] = 4; + frame [ 89*1024 + 300] = 7; + frame [300*1024 + 0] = 3; + + test.SetInternalGeneratorFrame(frame); + + test.SetSpotFinderParameters(0, 10); + + REQUIRE_NOTHROW(test.StartAction(x)); + REQUIRE_NOTHROW(test.WaitForActionComplete()); + + REQUIRE(test.OutputStream().size() == 1); + + JFJochProtoBuf::AcquisitionDeviceStatistics device_statistics; + REQUIRE_NOTHROW(test.SaveStatistics(x, device_statistics)); + REQUIRE(device_statistics.bytes_received() == 128 * nmodules * JUNGFRAU_PACKET_SIZE_BYTES); + + auto imageBuf = test.GetDeviceOutput(0, 0)->pixels; + REQUIRE(memcmp(imageBuf, frame.data(), RAW_MODULE_SIZE * sizeof(uint16_t)) == 0); + + auto spot_finder_result = test.GetDeviceOutput(0, 0)->spot_finding_result; + REQUIRE (spot_finder_result.strong_pixel_count == 2); + REQUIRE (spot_finder_result.snr_threshold == 10 * 4); + REQUIRE (spot_finder_result.count_threshold == 0); + REQUIRE (spot_finder_result.strong_pixel[0] == (1<<0)); + REQUIRE (spot_finder_result.strong_pixel[(89*1024 + 300) / 8] == (1<<4)); // 300 % 8 == 4 } \ No newline at end of file