Difference between the stated algorithm and other sources

[FloCD]

Hello gfoidl,

as I am also trying to implement the Swinging door algorithm I was looking through your very helpful graphics and descriptions of the algorithm. As I was implementing it I was verifying my code against the output of your code ("erregerspannung.csv without deadband") and was also reading other resources. One thing was puzzleing for me though. You are stating that:

Compared to other resources like https://softwaredocs.weatherford.com/cygnet/94/Content/Topics/History/CygNet%20Swinging%20Door%20Compression.htm or https://www.youtube.com/watch?v=89hg2mme7S0 I thought that only the last snapshot (the datapoint for the beginning for the new swinging doors) is saved and not the next datapoint which exceeds the spawned area by the swinging doors. I would be very grateful if you could clarify that a bit. Another interesting thing to know would be how you verified the correctness of your algorithm.

Thanks in advance!

All the best

FloCD

[gfoidl]

Thanks for your report!

While I don't remember why it's done that way, I believe you found a "bug". The " because it's not a correctness bug, but the current implementation misses some compression possibilities. Maybe I just used the wrong test data to see this (or rather didn't see this).

For tuples like

(0, 1)
(1, 1.1)
(2, 1.2)
(3, 1.6)
(4, 2)
(5, 2)
(6, 2)
(7, 1.2)

it looks like:

So there are too many points in the output, that aren't needed to represent the raw-graph in a compressed form.

The problematic location is code is

DataCompression/source/gfoidl.DataCompression/Compression/SwingingDoorCompression/SwingingDoorCompressionIterator.cs

Lines 45 to 49 in 6dc1da9

	protected void OpenNewDoor(in DataPoint incoming)
	{
	_lastArchived = incoming;
	_slope = s_newDoor;
	}

which a) records the incoming value (6 in the image above), and b) opens a completely new door (+∞, -∞).
Correct would be to don't record the incoming value, and just construct the door from the last recorded value with the given compression deviation.

I'll need to fix this (but can't promise when it's done).

BTW: there's a typo in the description too.

[FloCD]

Hi gfoidl,

thanks for your fast reply! Do you have a more extensive data sample to cross check the behaviour of the swinging door compression (raw data and compressed data)?

All the best

Florian

[gfoidl]

No, I didn't have. These are constructed manually with as less data as possible (to make debugging, etc easier).

In production use I verify the graphs of raw and compressed for same samples -- also to determine the compressionDeviation.
So far I didn't spot this flaw you brought up. So thanks for reaching out.

[FloCD]

No problem! If I can generate some sample data, I will share it with you if you want. By the way: as I was reading through your examples and your descriptions for the swinging door algorithm it would be quite helpful, if you also state the compressionDeviation you used. (For the small 10 datapoints examples)

[gfoidl]

If I can generate some sample data, I will share it with you if you want.

That would be great. 👍🏻
With the fix (#60) I plan to make adding new sample data to use for tests easier. So that a whole test-suite could be run to cross-check.

state the compressionDeviation you used

Good point. Thanks!

[FloCD]

Hi gfoidl,

as you have merged a pull request, I wanted to ask you if you have fixed the issue. Then I could cross check it with my algorithm.

All the best

Florian

[gfoidl]

PR for the fix is still open, and work in progress.
This week I'm out of office, so will complete it next week.
A cross check would be great. I will ping you then. Thanks!

[FloCD]

Thanks for the update!

All the best

Florian

[gfoidl]

I don't remember why it's done that way

Now I remember. For the implementation of deadband and swinging door I wanted to share as much code as possible, so there's a common base class. Deadband got implemneted first, and there one needs to store the last snapshot and the incoming value. After deadband was done, swinging door got implemented, but there the incoming value shouldn't be stored and I missed this somehow.

The PR for the fix (got a bit bigger than wanted) is ready, just need to polish some things.

[FloCD]

Hi gfoidl,

thanks for the update! I will try to check it as soon as possible!

All the best

Florian

[gfoidl]

@FloCD I merged the PR, as I need the fix for work-projects too.

If there's still too much data, etc. please re-open the issue or create a new one.

Thanks for pointing this out!

[FloCD]

Hi gfoidl,

thats no problem! Thanks for your fast answer and reaction! I am currently working on the cross check, as soon as I get a positive result, I will post it here.

All the best

FloCD