Compress record FIFO/LIFO not working

I have pached modules/ca/database/src/std/rec/compressRecord.c

In get_array_info was a query if prec-balg == bufferingALF_FIFO and if true
a new offset was computed. The correct sorting into bptr but already in put_value depends on FIFO or LIFO. One should not do that twice.

> ############################################################################
> ## EPICS R7.0.1.2-DEV
> ## EPICS Base built Apr 9 2018
> ############################################################################
>
> compress with ALG "N to 1 Average”.
>
> db-file:
>
> record(waveform, "$(P):TimeScale") {
> field(FTVL, "DOUBLE")
> field(NELM, "2500")
> field(EGU, "ns")
> field(FLNK, "$(P):TimeScale:mid")
> field(TPRO, "1")
> }
>
> record(compress, "$(P):TimeScale:mid") {
> field(DESC, "Time scale (mid res)")
> field(INP, "$(P):TimeScale")
> field(ALG, "N to 1 Average")
> field(N, "5")
> field(NSAM, "500")
> field(EGU, "ns")
> field(TPRO, "1")
> }
>
before the patch:

caget dumm:TimeScale 2500 0 10 20 30 40 50 60 70 80 90 100 110 120 130 140 150 160 170 180 190 200 210 220 230 240 250 260 270 280 290 300 310 320 330 340 350 360 370 380 390 400 410 420 430 440 450 460 470 480 490 500 510 520 530 540 550 560 570 580 590 600 610 620 630 640 650 660 670 680 690 700 710 720 730 740 750 760 770 780 790 800 810 820 830 840 850 860 870 880 890 900 910 920 930 940 950 960 970 980 990 1000 1010 1020 1030 1040 1050 1060 1070 1080 1090 1100 1110 1120 1130 1140 1150 1160 1170 1180 1190 1200 1210 1220 1230 1240 1250 1260 1270 1280 1290 1300 1310 1320 1330 1340 … 24960 24970 24980 24990

caget dumm:TimeScale:mid 500 14820 14870 14920 14970 15020 15070 15120 15170 15220 15270 15320 15370 15420 15470 15520 15570 15620 15670 15720 15770 15820 15870 15920 15970 16020 16070 16120 16170 16220 16270 16320 16370 16420 16470 16520 16570 16620 16670 16720 16770 16820 16870 16920 16970 17020 17070 17120 17170 17220 17270 17320 17370 17420 17470 17520 17570 17620 17670 17720 17770 17820 17870 17920 17970 18020 18070 18120 18170 18220 18270 18320 18370 18420 18470 18520 18570 18620 18670 18720 18770 18820 18870 18920 18970 19020 19070 19120 19170 19220 19270 19320 19370 19420 19470 19520 19570 19620 19670 19720 19770 19820 19870 19920 19970 20020 20070 20120 20170 20220 20270 20320 20370 20420 20470 20520 20570 20620 20670 20720 20770 20820 20870 20920 20970 21020 21070 21120 21170 21220 21270 21320 21370 21420 21470 21520 21570 21620 21670 21720 21770 21820 21870 21920 21970 22020 22070 22120 22170 22220 22270 22320 22370 22420 22470 22520 22570 22620 22670 22720 22770 22820 22870 22920 22970 23020 23070 23120 23170 23220 23270 23320 23370 23420 23470 23520 23570 23620 23670 23720 23770 23820 23870 23920 23970 24020 24070 24120 24170 24220 24270 24320 24370 24420 24470 24520 24570 24620 24670 24720 24770 24820 24870 24920 24970 20 70 120 170 220 270 320 370 420 470 520 570 620 670 720 770 820 870 920 970 1020 1070 1120 1170 1220 1270 1320 1370 1420 1470 1520 1570 1620 1670 1720 1770 1820 1870 1920 1970 2020 2070 2120 2170 2220 2270 2320 2370 2420 2470 2520 2570 2620 2670 2720 2770 2820 2870 2920 2970 3020 3070 3120 3170 3220 3270 3320 3370 3420 3470 3520 3570 3620 3670 3720 3770 3820 387...

this patch works also for me.
Thank you very much for your help Heinz!

Patch works, RL to commit.

I'm not convinced this patch is quite right yet. After applying it I'm seeing zeros instead of the values fetched from the input. I used a slightly simpler database and set ring.SCAN to "1 second" by hand after connecting the camonitor:

record(calc,"counter") {
 field(CALC, "VAL+1")
}
record(compress,"ring") {
# field(SCAN, "1 second")
 field(NSAM, "10")
 field(ALG, "Circular Buffer")
 field(INP, "counter PP")
# field(BALG, "FIFO Buffer")
 field(BALG, "LIFO Buffer")
 field(PREC, "1")
}

This is the camonitor output when ring.BALG is "LIFO Buffer":

ring 2018-06-26 16:37:09.663332 1
ring 2018-06-26 16:37:10.663222 2 2 1
ring 2018-06-26 16:37:11.663231 3 3 2 1
ring 2018-06-26 16:37:12.663253 4 4 3 2 1
ring 2018-06-26 16:37:13.663229 5 5 4 3 2 1
ring 2018-06-26 16:37:14.663222 6 6 5 4 3 2 1
ring 2018-06-26 16:37:15.663239 7 7 0 0 0 0 6 5
ring 2018-06-26 16:37:16.663247 8 8 7 0 0 0 0 6 5
ring 2018-06-26 16:37:17.663212 9 9 8 7 0 0 0 0 6 5
ring 2018-06-26 16:37:18.663218 10 10 9 8 7 0 0 0 0 6 5
ring 2018-06-26 16:37:19.663245 10 11 10 9 8 7 0 0 0 0 6
ring 2018-06-26 16:37:20.663221 10 12 11 10 9 8 7 0 0 0 0
ring 2018-06-26 16:37:21.663227 10 13 0 0 0 0 12 11 10 9 8
ring 2018-06-26 16:37:22.663223 10 14 13 0 0 0 0 12 11 10 9
ring 2018-06-26 16:37:23.663220 10 15 14 13 0 0 0 0 12 11 10
ring 2018-06-26 16:37:24.663224 10 16 15 14 13 0 0 0 0 12 11
ring 2018-06-26 16:37:25.663230 10 17 16 15 14 13 0 0 0 0 12
ring 2018-06-26 16:37:26.663220 10 18 17 16 15 14 13 0 0 0 0
ring 2018-06-26 16:37:27.663257 10 19 0 0 0 0 18 17 16 15 14
ring 2018-06-26 16:37:28.663227 10 20 19 0 0 0 0 18 17 16 15

This is the camonitor output when ring.BALG is "FIFO Buffer":

ring 2018-06-26 16:33:59.111461 0
ring 2018-06-26 16:34:00.111415 2 0 0
ring 2018-06-26 16:34:01.111395 3 0 0 0
ring 2018-06-26 16:34:02.111400 4 0 0 0 0
ring 2018-06-26 16:34:03.111396 5 0 0 0 0 0
ring 2018-06-26 16:34:04.111428 6 0 0 0 0 1 2
ring 2018-06-26 16:34:05.111413 7 0 0 0 1 2 3 4
ring 2018-06-26 16:34:06.111410 8 0 0 1 2 3 4 5 6
ring 2018-06-26 16:34:07.111398 9 0 1 2 3 4 5 6 7 8
ring 2018-06-26 16:34:08.111426 10 1 2 3 4 5 6 7 8 9 10

For the FIFO case it works Okay beyond the rows shown, whereas for the LIFO case I always get 4 adjacent zero values in the buffer, e.g:

ring 2018-06-26 16:45:31.145797 10 199 0 0 0 0 198 197 196 195 194
ring 2018-06-26 16:45:32.145824 10 200 199 0 0 0 0 198 197 196 195
ring 2018-06-26 16:45:33.145803 10 201 200 199 0 0 0 0 198 197 196
ring 2018-06-26 16:45:34.145786 10 202 201 200 199 0 0 0 0 198 197
ring 2018-06-26 16:45:35.145771 10 203 202 201 200 199 0 0 0 0 198
ring 2018-06-26 16:45:36.145814 10 204 203 202 201 200 199 0 0 0 0
ring 2018-06-26 16:45:37.145809 10 205 0 0 0 0 204 203 202 201 200

Scrap the FIFO buffer results above, I was using a version of Base that had some other changes which affected those results. However when using the real Base 3.16 branch with the patch applied and configured as a LIFO buffer I'm still seeing the zeros:

ring 2018-06-26 17:08:55.617463 1
ring 2018-06-26 17:08:56.617419 2 2 1
ring 2018-06-26 17:08:57.617387 3 3 2 1
ring 2018-06-26 17:08:58.617379 4 4 3 2 1
ring 2018-06-26 17:08:59.617390 5 5 4 3 2 1
ring 2018-06-26 17:09:00.617387 6 6 5 4 3 2 1
ring 2018-06-26 17:09:01.617384 7 7 0 0 0 0 6 5
ring 2018-06-26 17:09:02.617382 8 8 7 0 0 0 0 6 5
ring 2018-06-26 17:09:03.617416 9 9 8 7 0 0 0 0 6 5
ring 2018-06-26 17:09:04.617392 10 10 9 8 7 0 0 0 0 6 5
ring 2018-06-26 17:09:05.617406 10 11 10 9 8 7 0 0 0 0 6
ring 2018-06-26 17:09:06.617382 10 12 11 10 9 8 7 0 0 0 0
ring 2018-06-26 17:09:07.617435 10 13 0 0 0 0 12 11 10 9 8
ring 2018-06-26 17:09:08.617383 10 14 13 0 0 0 0 12 11 10 9
ring 2018-06-26 17:09:09.617432 10 15 14 13 0 0 0 0 12 11 10
ring 2018-06-26 17:09:10.617419 10 16 15 14 13 0 0 0 0 12 11
ring 2018-06-26 17:09:11.617403 10 17 16 15 14 13 0 0 0 0 12
ring 2018-06-26 17:09:12.617410 10 18 17 16 15 14 13 0 0 0 0
ring 2018-06-26 17:09:13.617388 10 19 0 0 0 0 18 17 16 15 14
ring 2018-06-26 17:09:14.617442 10 20 19 0 0 0 0 18 17 16 15

Hi Andrew,

Ok, this happens with nsam < 16 :-(

In the code for lifo-mode there is line

offset = (offset-1) % nsam;

offset is epicsUInt32.

that does not give comprehensible results.

e.g.
dumm.c

#include <stdio.h>
#include <stdint.h>

int main(int argc, char *argv[])
{

uint32_t offset;

int nsam1 = 10;
int nsam2 = 16;

offset = 0;
offset = (offset-1) % nsam1;
printf(" offset 1 = %d\n", offset);

offset = 0;
offset = (offset-1) % nsam2;
printf(" offset 2 = %d\n", offset);
}

h1:~ heinz$ ./dumm
 offset 1 = 5
 offset 2 = 15

I could write
if (offset == 0) offset = nsam-1

I found no explanation why from 16 it is right.
Have you ever seen this elsewhere?

Heinz
------------------------------------------------------------------------------
Fritz-Haber-Institut | Phone: (+49 30) 8413-4270
Heinz Junkes | Fax (G3+G4): (+49 30) 8413-5900
Faradayweg 4-6 |
D - 14195 Berlin | E-Mail: <email address hidden>
------------------------------------------------------------------------------

> On 27. Jun 2018, at 00:04, Andrew Johnson <email address hidden> wrote:
>
> I'm not convinced this patch is quite right yet. After applying it I'm
> seeing zeros instead of the values fetched from the input. I used a
> slightly simpler database and set ring.SCAN to "1 second" by hand after
> connecting the camonitor:
>
> record(calc,"counter") {
> field(CALC, "VAL+1")
> }
> record(compress,"ring") {
> # field(SCAN, "1 second")
> field(NSAM, "10")
> field(ALG, "Circular Buffer")
> field(INP, "counter PP")
> # field(BALG, "FIFO Buffer")
> field(BALG, "LIFO Buffer")
> field(PREC, "1")
> }
>
> This is the camonitor output when ring.BALG is "LIFO Buffer":
>
> ring 2018-06-26 16:37:09.663332 1
> ring 2018-06-26 16:37:10.663222 2 2 1
> ring 2018-06-26 16:37:11.663231 3 3 2 1
> ring 2018-06-26 16:37:12.663253 4 4 3 2 1
> ring 2018-06-26 16:37:13.663229 5 5 4 3 2 1
> ring 2018-06-26 16:37:14.663222 6 6 5 4 3 2 1
> ring 2018-06-26 16:37:15.663239 7 7 0 0 0 0 6 5
> ring 2018-06-26 16:37:16.663247 8 8 7 0 0 0 0 6 5
> ring 2018-06-26 16:37:17.663212 9 9 8 7 0 0 0 0 6 5
> ring 2018-06-26 16:37:18.663218 10 10 9 8 7 0 0 0 0 6 5
> ring 2018-06-26 16:37:19.663245 10 11 10 9 8 7 0 0 0 0 6
> ring 2018-06-26 16:37:20.663221 10 12 11 10 9 8 7 0 0 0 0
> ring 2018-06-26 16:37:21.663227 10 13 0 0 0 0 12 11 10 9 8
> ring 2018-06-26 16:37:22.663223 10 14 13 0 0 0 0 12 11 10 9
> ring 2018-06-26 16:37:23.663220 10 15 14 13 0 0 0 0 12 11 10
> ring 2018-06-26 16:37:24.663224 10 16 15 14 13 0 0 0 0 12 11
> ring 2018-06-26 16:37:25.663230 10 17 16 15 14 13 0 0 0 0 12
> ring 2018-06-26 16:37:26.663220 10 18 17 16 15 14 13 0 0 0 0
> ring 2018-06-26 16:37:27.663257 10 19 0 0 0 0 18 17 16 15 14
> ring 2018-06-26 16:37:28.663227 10 20 19 0 0 0 0 18 17 16 15
>
> This is the camonitor output when ring.BALG is "FIFO Buffer":
>
> ring 2018-06-26 16:33:59.111461 0
> ring 2018-06-26 16:34:00.111415 2 0 0
> ring 2018-06-26 16:34:01.111395 3 0 0 0
> ring 2018-06-26 16:34:02.111400 4 0 0 0 0
> ring 2018-06-26 16:34:03.111396 5 0 0 0 0 0
> ring 2018-06-26 16:34:...

I don't quite understand...

For an unsigned 32bit integer, 0-1 = 4294967295

4294967295 mod 10 = 5 (4294967295 / 10 = 429496729; 429496729 * 10 = 4294967290; remainder = 5)

4294967295 mod 16 = 15 (4294967295 / 16 = 268435455; 268435455 * 16 = 4294967280; remainder = 15)

What is not comprehensible with those results?

Of course you are right. I just wanted to try that out now. I just did not know the exact "big number" ;-)

I will insert the query to <= 0. Then it should work.

Heinz

> On 11. Jul 2018, at 21:18, Ralph Lange <email address hidden> wrote:
>
> I don't quite understand...
>
> For an unsigned 32bit integer, 0-1 = 4294967295
>
> 4294967295 mod 10 = 5 (4294967295 / 10 = 429496729; 429496729 * 10 =
> 4294967290; remainder = 5)
>
> 4294967295 mod 16 = 15 (4294967295 / 16 = 268435455; 268435455 * 16 =
> 4294967280; remainder = 15)
>
> What is not comprehensible with those results?
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1743795
>
> Title:
> Compress record FIFO/LIFO not working
>
> Status in EPICS Base:
> In Progress
> Status in EPICS Base 3.16 series:
> In Progress
> Status in EPICS Base 7.0 series:
> In Progress
>
> Bug description:
> Reported by Daniel Schick <daniel.schick_AT_mbi-berlin.de>:
>
> I am currently using epics base 3.16.1 and I have a problem with the compress record used as ring buffer.
> In both, Epics 3.16.1 and 7.0 the update of the ring buffer is not FIFO or LIFO but happens somewhere in the middle of the array.
> When I use the latest 3.15 branch everything works fine.
>
> Here is an example of my test db-file and the output from camonitor:
>
> record(calc,"counter") {
> field(CALC,"VAL+1")
> field(SCAN,"1 second")
> }
>
> record(compress,"ring") {
> field(DESC, "test")
> field(NSAM,"10")
> field(ALG,"Circular Buffer")
> field(INP,"counter")
> field(SCAN,"1 second")
> field(BALG,"LIFO")
> field(PREC,"1")
> }
>
> with epics 3.15 it is fine:
>
> ~ $ camonitor ring
> ring *** Not connected (PV not found)
> ring 2018-01-08 12:55:16.628324 8 0 1 2 3 4 5 6 7
> ring 2018-01-08 12:55:17.628331 9 0 1 2 3 4 5 6 7 8
> ring 2018-01-08 12:55:18.628323 10 0 1 2 3 4 5 6 7 8 9
> ring 2018-01-08 12:55:19.628330 10 1 2 3 4 5 6 7 8 9 10
> ring 2018-01-08 12:55:20.628326 10 2 3 4 5 6 7 8 9 10 11
> ring 2018-01-08 12:55:21.628331 10 3 4 5 6 7 8 9 10 11 12
> ring 2018-01-08 12:55:22.628327 10 4 5 6 7 8 9 10 11 12 13
> ring 2018-01-08 12:55:23.628324 10 5 6 7 8 9 10 11 12 13 14
> ring 2018-01-08 12:55:24.628325 10 6 7 8 9 10 11 12 13 14 15
> ring 2018-01-08 12:55:25.628332 10 7 8 9 10 11 12 13 14 15 16
> ring 2018-01-08 12:55:26.628326 10 8 9 10 11 12 13 14 15 16 17
> ring 2018-01-08 12:55:27.628330 10 9 10 11 12 13 14 15 16 17 18
> ring 2018-01-08 12:55:28.628323 10 10 11 12 13 14 15 16 17 18 19
> ring 2018-01-08 12:55:29.628325 10 11 12 13 14 15 16 17 18 19 20
> ring 2018-01-08 12:55:30.628327 10 12 13 14 15 16 17 18 19 20 21
> ring 2018-01-08 12:55:31.628331 10 13 14 15 16 17 18 19 20 21 22
> ring 2018-01-08 12:55:32.628327 10 14 15 16 17 18 19 20 21 22 23
>
>
> with epics 3.16 and 7.0 I get the weird update behaviour:
>
> ~ $ camonitor ring
> ring *** Not connected (PV not found)
> ring 2018-01-08 14:23:50.889595 2 0 1
> ring 2018-01-08 14:23:51.889598 3 0 1 2
> ring 2018-01-08 14:23:52.889594 4 0 1 2 3
> ring 2018-01-08 14:23:53.889596 5 0 1 2 3 4
> ring 201...

The attached patch also includes a check in put_value for offset == 0 that in LIFO mode the decrement will be set to nsam -1 (not MAX_UINT % nsam) which leads to

offset (==0)

-1 mod 1 => 0
-1 mod 2 => 1
-1 mod 3 => 0
-1 mod 4 => 3
-1 mod 5 => 0
-1 mod 6 => 3
-1 mod 7 => 3
-1 mod 8 => 7
-1 mod 9 => 3
-1 mod 10 => 5
-1 mod 11 => 3
-1 mod 12 => 3
-1 mod 13 => 8
-1 mod 14 => 3
-1 mod 15 => 0
-1 mod 16 => 15
-1 mod 17 => 0
-1 mod 18 => 3
-1 mod 19 => 5
-1 mod 20 => 15
-1 mod 21 => 3
-1 mod 22 => 3
-1 mod 23 => 11
-1 mod 24 => 15
-1 mod 25 => 20
-1 mod 26 => 21
-1 mod 27 => 21
-1 mod 28 => 3
-1 mod 29 => 15
-1 mod 30 => 15
-1 mod 31 => 3

Could you add a comment expressing what the code tries to accomplish?
Without more context I have trouble as well following your argumentation as understanding the suggested fix.

(Might also just be the heat.)

In the function put_value in the compressRecord there is a query whether one is in LIFO or FIFO mode.

In the current code it looks like this:

if(!fifo)
   offset = (offset -1) %nsam;

Since offset is of type epicsUint32, this line at offset == 0 leads to the following action

offset = 4294967295 mod nsam;

But it is intended that offset is set to nsam -1.
That's why I changed it to

if(!fifo)
    if(offset)
        offset = (offset -1) % nsam;
    else offset = nsam -1;

Heinz is right. The existing code would work correctly only if

(a) it were a signed integer, which it isn't, and

(b) the C % operator were defined as "modulo" in the mathematical sense i.e. always resulting in a value between 0 and (rhs-1), which it doesn't have to be, see https://stackoverflow.com/questions/4003232/how-to-code-a-modulo-operator-in-c-c-obj-c-that-handles-negative-numbers

Thus another correct fix is

if(!fifo)
   offset = (offset + nsam - 1) % nsam;

This change fixed the LIFO buffer case (thanks!), but you didn't actually test a real compress record in FIFO buffer mode when it is first filling up. I was still getting zeros while the buffer was first filling up:

ring <undefined> UDF INVALID
ring 2018-07-19 00:00:01.639170 0
ring 2018-07-19 00:00:03.639906 2 0 0
ring 2018-07-19 00:00:05.639230 3 0 0 0
ring 2018-07-19 00:00:07.639252 4 0 0 0 0
ring 2018-07-19 00:00:09.639243 5 0 0 0 0 0
ring 2018-07-19 00:00:11.639126 6 0 0 0 0 1 2
ring 2018-07-19 00:00:13.639223 7 0 0 0 1 2 3 4
ring 2018-07-19 00:00:15.639223 8 0 0 1 2 3 4 5 6
ring 2018-07-19 00:00:17.639224 9 0 1 2 3 4 5 6 7 8
ring 2018-07-19 00:00:19.639215 10 1 2 3 4 5 6 7 8 9 10
ring 2018-07-19 00:00:21.639171 10 2 3 4 5 6 7 8 9 10 11
ring 2018-07-19 00:00:23.639224 10 3 4 5 6 7 8 9 10 11 12
ring 2018-07-19 00:00:25.639234 10 4 5 6 7 8 9 10 11 12 13
ring 2018-07-19 00:00:27.639214 10 5 6 7 8 9 10 11 12 13 14

The result also failed tests 11, 19 and 27 of the compressTest in src/std/rec/test, for the same reason as the results above.

I worked out what the final piece of the puzzle was though; the original conditional code in get_array_info() is almost right and should not be removed, but it also needed the fix for when (off - nuse) goes negative. I used Ben's version, and the result passes the tests.

Committed and pushed to the 3.16 branch, now I can go to bed. (01:15am)