Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Online Facility Location #226

Closed
wants to merge 3 commits into from
Closed

Conversation

axmmisaka
Copy link
Collaborator

@axmmisaka axmmisaka commented Sep 1, 2023

Action item

  • Verify that this modification is correct and fits the original intention when Prof. Dr. @hokeun wrote it

Problem

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

Originally posted by @axmmisaka in #223 (comment)

…make connections

0. Added _addChild and _addSibling necessary for mutation APIs
1. Solved issues related to hierarchy and ownership.
@axmmisaka axmmisaka changed the base branch from mutation/addrelatives to mutation/full September 7, 2023 04:21
@axmmisaka axmmisaka mentioned this pull request Sep 7, 2023
9 tasks
We were connecting an outport to an outport in order to ensure that these two ports have the same output, which is an...... interesting way of achieving this goal. The simple hacky way to fix this is to ensure that these two outports are updated at the same time.
@axmmisaka
Copy link
Collaborator Author

This is fixed in #232, closing

@axmmisaka axmmisaka closed this Oct 3, 2023
@axmmisaka axmmisaka deleted the mutation/fix-facloc branch October 16, 2023 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant