Refactor Reactor::{connect, connectMulti, canConnect}

[axmmisaka]

0. canConnect doesn't experience situations where throwing Error is required.
1. canConnect is solely for testing and should not throw.
2. connect and connectMulti should throw. Simply return result from canConnect and throw in functions that calls them.
3. A mutation referencing a port for connection/disconnection should NOT result in any causality change per se between the port and the mutation

• Refactor tests
• Add descriptions for the enum members
• Implement ConnectablePort
  Note: this task ended up being a rather insane one - I told @lhstrh on Friday that I will make some slight restructuring to make it more sane, but it turns out that my attempts are rather futile and I will commit the rough and crazy one I made on Friday. The issue is that ConnectablePort must still be able to be a valid Variable and cannot be a copy/wrapper that could be passed into connect. After all, connect taking an IOPort is too big to be changed as of now. So my approach is to make a simple wrapper to bypass the causality interface update. I think a more radical solution (such as making connect only take ConnectablePort, and unwrap/get the IOPort inside of it) deserves another branch/PR.
• Fix connection bugs (which is why jest is not passing)
  • Do not automatically fail direct feedthrough if it does not create any causality issue
  • Do not limit the scope on local check if causality interface has changed Always do global check
• Discuss with @lhstrh if in-mutation casting should be allowed

[axmmisaka]

After discussion today with @lhstrh we discovered that the current logic of detecting causality interface/direct feedthrough is a bit problematic. I will keep as-is as of now (i.e. no change on enum type/return type) and add tests to make sure that the causality interface issue is resolved first.

This is honestly something that needs to be done a long time ago but it is really seriously overdue......

[axmmisaka]

@lhstrh :
For reference, the decision regarding code generator:
If we want to connect a port in one microstep as a source, and write to it (or as a destination, but the current implementation wouldn't allow us to read from it I'm afraid), we have two approaches to do this, and they will have the same effect on the causality interface:

Or, should we NOT allow this do be done at all in one tag? This will be logically hard to achieve, I'm afraid, as this way we will need to record if a port is newly connected in the current tag.

1. Declare two Variables:

this.addMutation(
  [this.trig],
  [this.writable(this.myport), this.myport.asConnectable(), this.destport.asConnectable()],
  function (this, wport, cport, dest) {
    this.connect(cport, dest);
    wport.set(123);
  }
);

2. Declare one Variable and cast inside:

this.addMutation(
  [this.trig],
  [this.writable(this.myport), this.destport.asConnectable()],
  function (this, port, dest) {
    this.connect(dest, cport);
    port.getPort().asConnectable().set(123);
  }
);

Here are some approaches, but I'm still thinking about tradeoff/reason to allow only one:

1. Allow both -> do nothing
2. Allow only 1 -> mimic asWritable() and require the reactor key for it to be casted
3. Allow only 2 -> check for duplication in reaction variables. We currently don't need to check for duplication, because if there's conflict (e.g. as a connection destination and an InPort to be read, a cycle will be formed in the causality graph).

[lhstrh]

One problem with the diff is that all of the logging stuff is in it too. I think mutation/full should have master merged into it so that the diff shown in this PR is smaller.

[axmmisaka]

One problem with the diff is that all of the logging stuff is in it too. I think mutation/full should have master merged into it so that the diff shown in this PR is smaller.

Fixed! Sorry

[lhstrh]

While there may be some rough edges in the PR, it's generally a solid improvement, and the tests are running fine, so we should probably just go ahead and merge this...