Sodium switchS semantics vs Typescript implementation

[cubuspl42]

From Appendix E:

E.5.6 SwitchS

[...]

TEST CASES

[...]

From Typescript implementation unit tests:

test('should test switchS()', (done) => {
  class SS {
    constructor(a: string, b: string, sw: string) {
      this.a = a;
      this.b = b;
      this.sw = sw;
    }

    a: string;
    b: string;
    sw: string;
  }

  const sss = new StreamSink<SS>(),
    sa = sss.map(s => s.a).rename("sa"),
    sb = sss.map(s => s.b).rename("sb"),
    csw_str = sss.map(s => s.sw).rename("sss.map").filterNotNull().hold("sa"),
    csw = csw_str.map(lambda1(sw => sw == "sa" ? sa : sb, [sa, sb])).rename("csw_str.map"),
    so = Cell.switchS(csw),
    out: string[] = [],
    kill = so.listen(x => {
      out.push(x);
      if (out.length === 9) {
        done();
      }
    });

  sss.send(new SS("A", "a", null));
  sss.send(new SS("B", "b", null));
  sss.send(new SS("C", "c", "sb")); // <-- HERE
  sss.send(new SS("D", "d", null));
  sss.send(new SS("E", "e", "sa")); // <-- HERE
  sss.send(new SS("F", "f", null));
  sss.send(new SS("G", "g", "sb")); // <-- HERE
  sss.send(new SS("H", "h", "sa")); // <-- HERE
  sss.send(new SS("I", "i", "sa")); // <-- HERE
  kill();

  expect(out).toEqual(["A", "B", "C", "d", "e", "F", "G", "h", "I"]);
});

Semantic states that the event from the new stream is preferred, while the Typescript version picks the event from the old stream. Is there a good reason for it, or is it a bug?

[cubuspl42]

I checked and it's same in the Java version. Am I reading the sematics chart correctly?
@the-real-blackh

[clinuxrulz]

I fell for the same trap once.

In the expression:
let ca = sa.hold(0);

When sa fires ca only takes on the new value AFTER the current transaction. That is to make sure that there is consistency between snapshots.

So in the test case where it looks like the cell containing the stream changed value at the same time the old stream fired. The outer cell itself does not change value until after the current transaction. So the test case is actually correct.

[cubuspl42]

If the test is correct, then is semantics incorrect? The chart itself handles only one case, and I didn't try to analyze/execute the Haskell code, but if I'm reading it correctly, it picks 'Y' when c switches to s2 at time 2, not 'c'.

I do know how hold works. But switchS, at least in Typescript, doesn't seem to be consistent at all. It picks the event from either the old or the new stream, depending on... I don't really know what. You can take a look:

Add a few tests for switchS

[clinuxrulz]

I have not yet looked at your additional test cases, but I'll try to explain what is meant to happen a bit better.
So both the tests and the semantics are correct. Its just the time at which the cell switches to the new value is not obvious in the code.

During a transaction no cells change value. Just after the end of the transaction their new values are updated. So in the case of switchS it behaves like the old inner stream until the next transaction (The value of cells changes between transactions).

Implementation wise Cells are made up of a current value and a stream of new values that it will take on after the transaction in which the stream fires.

Check out the hold_is_delay test for clarification.

In your first picture above see the vertical divider for s1 and s2 for the value of c is between transaction (t value) 1 and 2.

[cubuspl42]

I'm aware of the rules you mentioned. It would really make sense to me if switchS behaved consistently with the "delayed" ruleset. The only problem is that, as far as I can tell, it doesn't. Please check out the new tests when you have a spare moment. You don't need to run them, they all pass.

In your first picture above see the vertical divider for s1 and s2 for the value of c is between transaction (t value) 1 and 2.

I was afraid that this is the case... It's the main misunderstanding. I guess I know understand the idea behind this chart.

[clinuxrulz]

I'm looking at your 'should test switchS()', and the values outputed to "out" array are what I'd expect them to be. I do not see any problem with the test.

Can you explain what you expected to see? Maybe I'm looking in the wrong area.

  sss.send(new SS("A", "a", null));
  sss.send(new SS("B", "b", null));
  sss.send(new SS("C", "c", "sb"));
  sss.send(new SS("D", "d", null));
  sss.send(new SS("E", "e", "sa"));
  sss.send(new SS("F", "f", null));
  sss.send(new SS("G", "g", "sb"));
  sss.send(new SS("H", "h", "sa"));
  sss.send(new SS("I", "i", "sa"));
  sss.send(new SS(null, "j", "sb"));
  sss.send(new SS("K", null, "sa"));
  sss.send(new SS("L", null, "sb"));
  sss.send(new SS(null, "m", "sa"));

The third param passed to SS constructor is what it stream will be in use in the NEXT transaction there after (the next send line will witness the new value of the cell value that contains the stream). Where each send (unless wrapped in Transaction.run()) is a single transaction. I followed my eyes over it, and what I'd expect it to be matched what was suggested here:

expect(out).toEqual(["A", "B", "C", "d", "e", "F", "G", "h", "I", "L", "m"]);

I should use grammarly, my tone sound angry when I read this. But I'm not angry honest .

---

This test further down looks a bit funny:

test('should test switchS + Stream.map', (done) => {
  const sa = new StreamSink<number>();
  const out: number[] = [];

  const co = Cell.switchS(new Cell(
    new Stream<number>(),
    sa.map((a) => {
      return sa.map((a_) => a * a_);
    }),
  ));

You've got a Cell constructor taking two parameters, when the Cell constructor (publicly visible one) is meant to take only 1 parameter (the const cell).
If it was intended to use the private constructor for testing. Any lambda expressions containing sodium objects should use the lambda wrappers for the memory management to work.
I.E.

    sa.map((a) => {
      return sa.map((a_) => a * a_);
    }),

becomes

    sa.map(lambda1((a) => {
      return sa.map((a_) => a * a_);
    }, [sa])),

I'm actually a bit confused by the test 'should test switchS + Stream.map' at the moment. I'll look more into it later.

[clinuxrulz]

I'll quickly explain consistency between snapshots. It might help.

let sd = sa.snapshot(ca);
let se = sb.snapshot(ca);
let sf = sc.snapshot(ca);

if sa, sb and sc is fired at the same time during a transaction, then no matter how deep they are in the graph or what their ranks are. sd, se and sf will pass along the same value, which is the value of ca at the time of the transaction. And if ca is going to change, then it will only be after the current transaction.

So in short all snapshots that occur during a transaction will grab the same value of that ca.

The reason for this? The ability to accumulate state within a Cell. That is to have the state of a cell update itself in regards to its own state and the value of a fired stream.

E.g.

let slCounter = new StreamLoop();
let cCounter = slCounter.hold(0);
sl.loop(sIncrement.snapshot1(cCounter).map(x => x + 1));

[cubuspl42]

Thank you for the snapshot consistency explanation. I was aware of that mechanism.

If switchS preferred the value from the newer stream, as far as I can tell, it wouldn't really break snapshot consistency; it'd "only" break the cell updates undetectability, i.e. inability to extract a stream from a cell, which I find weaker from the snapshot consistency rule. And arguable.

Of course, it'd put switchS to the same league as the value and update operators, which Stephen considers "operational".

I now agree with you that 'should test switchS()' is compliant with the semantics; I was reading the chart in the wrong way.

It also means that ''should test switchS + Cell.map' is correct; ignore them both.

I didn't really care for memory leaks with these tests, they aren't meant to be merged with the upstream as-is. I added lambda1, so it's not considered a cause for the test passing.

I'm pretty sure that the 2-argument Cell constructor is a synonym for hold, but I still converted the test to explicit holds.

Please look again at 'should test switchS + value' and 'should test switchS + Stream.map' when you have time:

Add a few tests for switchS

[clinuxrulz]

There definitely seems to be a problem in the code somewhere
'should test switchS + Stream.map' should not fire two values in one transaction.

These lines in 'test switchS + Stream.map':

  sa.send(4);

  // 16 was produced by the new stream (old one was Never)
  expect(out).toEqual([16]);

I would of expected out to be empty because the old stream was wired up for that transaction and the new stream should be for the next transaction.

I wonder if it is something I messed up somewhere when trying to optimise sodium typescript. I'll look through the commit history and retry that test on an older version.

This commit might be the cause:

memory bounce in switchS/switchC fix

It's purpose was to stop the reference counts falling to zero, then returning to one. As it would suddenly free and reconstruct sodium object if it did.

When I get my hands on a computer I'll have a look.

[clinuxrulz]

OK. I had a little play and the fix is simple, just use Transaction.currentTransaction.last() to defer the graph change in switchS to occur between transactions (including an extra temporary dummy listener to keep memory management working too). But I'm curious to see if the same bug exists in the Java version, because the switchS code is nearly identical. Just need to find some more free time to have another play.