Skip to content

Port dependency/precedence logic when adding a mutation #223

Description

@axmmisaka

This is a problem which troubled me for a while and I discovered it might potentially be a rather major issue.

When adding a mutation, it is necessary to declare ports. We declare a port as an argument, as things are set up currently, either as an IOPort or as a WritablePort. They are, then, handled differently in recordDeps: either as dependency or precedence to the mutation.

if (a instanceof IOPort) {
this._dependencyGraph.addEdge(
a,
reaction as unknown as Reaction<Variable[]>
);
sources.add(a);
} else if (a instanceof MultiPort) {

} else if (a instanceof WritablePort) {
this._dependencyGraph.addEdge(
reaction as unknown as Reaction<Variable[]>,
a.getPort()
);
effects.add(a.getPort());
} else if (a instanceof WritableMultiPort) {

The issue is, they are able to be converted to one another inside of the mutation (by using this.getReactor().writable(port), or (port as unknown as WritablePort<number>).getPort()), and potentially create a causality loop. Unfortunately to do connection inside of a mutation, it appears that IOPort version is required, and if we want to connect AND write to a port then conversion is necessary (either pass in an WritablePort and convert to IOPort when connecting, or pass in an IOPort and convert to WritablePort).

Activity

  1. axmmisaka commented on Aug 29, 2023

    @axmmisaka
    CollaboratorAuthor

    Here's an example, a fix which makes Prof. Dr. @hokeun 's Online Facility Location benchmark working:

    4a230b6

    (See diff for more info)

    The corresponding runtime graph are as follow:

    Not working

    graph
    0["facilityLocation.0c4e7357[R0]"]
    1["facilityLocation.0c4e7357[M0]"]
    2["facilityLocation.0c4e7357[M1]"]
    3["facilityLocation.0c4e7357.InPort"]
    4["facilityLocation.0c4e7357.OutPort"]
    5["facilityLocation.0c4e7357.OutPort"]
    6["facilityLocation.0c4e7357.OutPort"]
    7["facilityLocation.0c4e7357.OutPort"]
    8["facilityLocation.0c4e7357.OutPort"]
    9["facilityLocation.0c4e7357.OutPort"]
    10["facilityLocation[M0]"]
    11["facilityLocation.0c4e7357.866cd7ec.OutPort"]
    1 --> 0
    10 --> 1
    0 --> 2
    3 --> 2
    2 --> 4
    2 --> 5
    2 --> 6
    2 --> 7
    2 --> 8
    2 --> 9
    11 -- "Problem here" --x 9
    
    Loading

    Error:

    Exception occurred in reaction: facilityLocation.dbdc9a01[M1]: Error: Destination port is already occupied.
    /home/nkagamihara/reactor-ts/src/core/reactor.ts:1125
          throw Error("Destination port is already occupied.");
                ^
    Error: Destination port is already occupied.
        at Quadrant.canConnect (/home/nkagamihara/reactor-ts/src/core/reactor.ts:1125:13)
        at _mutationSandbox.connect (/home/nkagamihara/reactor-ts/src/core/reactor.ts:446:26)
        at partition (/home/nkagamihara/reactor-ts/src/benchmark/FacilityLocation.ts:447:41)
        at _mutationSandbox.<anonymous> (/home/nkagamihara/reactor-ts/src/benchmark/FacilityLocation.ts:495:33)
        at Mutation.doReact (/home/nkagamihara/reactor-ts/src/core/reaction.ts:155:18)
        at FacilityLocation._react (/home/nkagamihara/reactor-ts/src/core/reactor.ts:2352:11)
        at FacilityLocation._next (/home/nkagamihara/reactor-ts/src/core/reactor.ts:2485:27)
        at FacilityLocation.<anonymous> (/home/nkagamihara/reactor-ts/src/core/reactor.ts:2581:16)
        at processImmediate (node:internal/timers:478:21)
    

    Working, but there's big error because we are only writing to it:

    graph
    0["facilityLocation.78c27ce7[R0]"]
    1["facilityLocation.78c27ce7[M0]"]
    2["facilityLocation.78c27ce7[M1]"]
    3["facilityLocation.78c27ce7.InPort"]
    4["facilityLocation.78c27ce7.OutPort"]
    5["facilityLocation.78c27ce7.OutPort"]
    6["facilityLocation.78c27ce7.OutPort"]
    7["facilityLocation.78c27ce7.OutPort"]
    8["facilityLocation.78c27ce7.OutPort"]
    9["facilityLocation.78c27ce7.OutPort"]
    10["facilityLocation[M0]"]
    11["facilityLocation.78c27ce7.bdba16f9.OutPort"]
    1 --> 0
    10 --> 1
    0 --> 2
    3 --> 2
    9 --> 2
    2 --> 4
    2 --> 5
    2 --> 6
    2 --> 7
    2 --> 8
    11 --> 9
    
    Loading

    This appears to be a bad example, as the issue is that the programme should not be written this way (that is, both could affect the outport which is not good). Nevertheless this opens the possibility of writing potentially malformed code.

  2. hokeun commented on Aug 29, 2023

    @hokeun
    Member

    Thanks for bringing this up!
    Please note that the Facility Location example was written a very long time ago on top of the older version of reactor-ts, which might be different from what we have now.

    Trying to understand the issue better, could you elaborate on what you meant by "the program should not be written this way" and what you meant by "both" that could affect the port?

  3. axmmisaka commented on Sep 1, 2023

    @axmmisaka
    CollaboratorAuthor

    Thanks for bringing this up! Please note that the Facility Location example was written a very long time ago on top of the older version of reactor-ts, which might be different from what we have now.

    Trying to understand the issue better, could you elaborate on what you meant by "the program should not be written this way" and what you meant by "both" that could affect the port?

    Sorry for my delayed reply.

    In the original code, in Quadrant's second mutation, toAccumulator (an OutPort) was the destination of two components:
    toNextAccumulator of Accumulator:

    const toAccumulatorOfQuadrant = (
    toAccumulator as unknown as WritablePort<Msg>
    ).getPort();
    // Connect Accumulator's output to Quadrant's output.
    this.connect(accumulator.toNextAccumulator, toAccumulatorOfQuadrant);

    and the mutation ([M1] itself:

    toAccumulator.set(
    new ConfirmExitMsg(
    0, // facilities
    supportCustomers.get().length, // supportCustomers
    1 // quadrantReactors
    )
    );

    I suppose, effectively, the first connection (which doesn't make much sense in the first place as it was connecting an OutPort to an OutPort), is to make a logic that "once toNextAccumulator is set, we also set toAccumulator"

    My current ugly fix is simply replicate this logic without making any additional connection by accessing a parent port from children: in Accumulator:

    [
    this.fromFirstQuadrant,
    this.fromSecondQuadrant,
    this.fromThirdQuadrant,
    this.fromFourthQuadrant
    ],
    [
    this.fromFirstQuadrant,
    this.fromSecondQuadrant,
    this.fromThirdQuadrant,
    this.fromFourthQuadrant,
    this.writable(this.toNextAccumulator),
    parent.writable(parent.toAccumulator),
    ],

    Then set both ports when one needs to be updated:

    toNextAccumulator.set(
    new ConfirmExitMsg(
    numFacilities + 1, // Add one for the facility itself.
    // (A quadrant with four children is considered as one facility in Akka-version implementation.)
    numCustomers,
    numQuadrantReactors + 1 // Add one for the quadrant reactor itself.
    )
    );
    parentToAccumulator.set(
    new ConfirmExitMsg(
    numFacilities + 1, // Add one for the facility itself.
    // (A quadrant with four children is considered as one facility in Akka-version implementation.)
    numCustomers,
    numQuadrantReactors + 1 // Add one for the quadrant reactor itself.
    )
    );

    I suppose this is not a good way to do it...... So I'm all ears

  4. axmmisaka commented on Sep 1, 2023

    @axmmisaka
    CollaboratorAuthor

    Nevertheless, outside of facloc, the issue is that a port could (if people are using reactor-ts with typescript, not lf+code generator) be converted to a writable port inside of a mutation, if the reactor where the mutation resides in has access to that port, without changing the causality interface and could potentially lead to causality loop, and I currently can't think of any good way to check this (unless we check the causality interface every time we do Reactor::writable?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions