# Some misconceptions about the "peak\_detector2" block **URL:** \ **Category:** GNU Radio\ **Created:** [April 10, 2015, 10:24pm UTC](https://www.ruby-forum.com/t/some-misconceptions-about-the-peak-detector2-block/241516 "2015-04-10T22:24:11Z")\ **Posts on this page:** 2\ **Page:** 1
**Author:** ![Detlef\_R](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/d/58f4c7/32.png) [@Detlef\_R](https://www.ruby-forum.com/u/Detlef_R)\ **Post date:** [April 10, 2015, 10:24pm UTC](https://www.ruby-forum.com/t/some-misconceptions-about-the-peak-detector2-block/241516/1 "2015-04-10T22:24:11Z")
Hi all, recently there has been some discussion regarding the peak\_detector2 block, both in the github/gnuradio (pull request 404) as well as in the issue tracker (issue 783). It is now well accepted that this block is buggy: there are cases the work function returns -1, which is a bug (see issue 783 on how to recreate this bug). I believe however that there is a DEEPER misconception about how this block works/should work that has resulted in some frustration on what an appropriate fix should be. In particular there is an insistence that an appropriate bug fix should pass the qa\_test of this block and it should be [in the spirit] of the existing algorithm. In the following I will explain why _passing the qa\_test is a consequence of the buggy behaviour_ of this block and NOT its feature. In addition I will suggest what a proper behaviour of this block should be, so that others who may want to write their own version of a peak detector find it useful. * * * So the peak\_detector block is very reasonable in its conception and its name is very informative and appropriate. It works as follows: Reads the input and keeps track of a running average (through a single-pole iir filter) When the current input crosses a threshold (= average \* a user-defined factor) upwards the block enters a search state, where it looks for the maximum value of the input over a window of user-defined length. This is clearly the intended behaviour of the block according to the documentation (I don’t know who the original author is…). * * * It should be obvious that this block is intended for a scenario where the inputs are coming continuously and the block is determining the peaks. A prototypical application is finding the peak after a MF with a training sequence in order to determine the start of a packet (BTW, there is some effort on creating a correlate and sync block, which is very relevant to this discussion). It should also be obvious that both the pole of the IIR filter, the threshold, AND the window should be properly selected per application (eg, for a training sequence of length N, it is appropriate to set the window ~N) otherwise the block will simply not work according to its specs. * * * So here are some questions and my suggested answers: Q1: How should this block behave when it has already crossed the threshold upwards and is still inside the user-defined search window but there is not enough input data to process? A1: Clearly the work function should return (consuming all inputs/outputs up to the most current peak–but not more) and hopefully the next time it is called, the scheduler would provide more samples so that the search over the remaining window is exhausted and the peak is reported. * * * Q2: And then, what if the scheduler does not give this block more input data? Maybe because (as in the qa\_test) this block is connected to a source that outputs a finite (=21 !) number of samples? A2: Since the intended use of this block is a continuous stream of data, the above situation is an inappropriate use of this block, and thus it cannot be considered a test case. This means that an appropriate test case should make sure that enough tail data is in the finite input so that the search window is exhausted. * * * Q3: What if the user insists in using the block in a way that is not intended, ie, by providing it with a finite (and small compared to the search window) number of inputs. Should there be a way that the block bails out? A3: The current implementation has this behaviour but it is a BUG not a feature: ERRONEOUSLY it decrements the window even if it does not return any data, so eventually the window becomes smaller than the available input and the block exits! This results in the block passing an ill-conceived qa\_test (where the window is set to 1000 for a peak occurring within 10 samples of the threshold crossing). However the intended use of the block was never this (according to the documentation). * * * From the above it should be clear that any fix of this block should NOT be in the spirit of the original one, nor not passing the existing qa\_test is a measure of its difference from the original block (at least from the stated intended behaviour of the original block). I attach here a clean modification of this block that has the intended behaviour. It DOES NOT pass (correctly) the inappropriate qa\_test, but it does pass more appropriately designed test(s) (also attached). ---
**Author:** ![Achilleas\_A](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/a/74df32/32.png) [@Achilleas\_A](https://www.ruby-forum.com/u/Achilleas_A)\ **Post date:** [April 13, 2015, 10:57pm UTC](https://www.ruby-forum.com/t/some-misconceptions-about-the-peak-detector2-block/241516/2 "2015-04-13T22:57:10Z")
Achilleas, I think you’ve completely failed to understand the issue from my perspective. I do NOT disagree that there is a bug in the code. I also do NOT disagree that most of what you’ve tried to do in the rewrite is the correct way to rethink the block. What I have a problem with is that you’ve provided me with a fix that breaks applications that used to run fine, including the QA test. As for the applications, there’s a really simple test you can perform. Apply the peak detector to a sine wave. In the current code, it finds the peak of the sine wave. With the new version, it does not just find that as the peak, but it outputs as though it’s found the peak for every length of the look-ahead value. This could be a valid design choice in a peak detector where given a window, always emit the highest value in the window. That is not what this block is supposed to do, nor is the new block behaving consistently in this manner. As for the QA test, the current tests presents a vector to the block and the block finds the correct peak. With the new code, it doesn’t complete. It just hangs. While it is true that stream-based blocks in GNU Radio expect a continuous stream of data, any block that simply fails to complete its processing when the rest of the flowgraph is done is a bug. Instead, we have hooks like set\_output\_multiple and overloading the forecast function that help us work with the scheduler to make sure everyone gets the right amount of data they require. In this case, you make a good argument that the block should look beyond it’s current window to see if the max is in fact reached. If that’s the case, then we need to have the block tell the scheduler this. If less data is passed because there is no more data to process and the flowgraph is shutting down, this block too must shut down. It will then do so without providing the right answer. So the QA test will fail – but it will complete and report failure in the data. Please understand the above points when reworking your fix. Thanks, Tom On Fri, Apr 10, 2015 at 4:22 PM, Achilleas A. \