Lane.ADD_GTU_EVENT lane in payload and event order

[WJSchakel]

The RoadSampler now abuses the source of an Lane.ADD_GTU_EVENT event to obtain the relevant lane. The lane has to become part of the payload.

Furthermore, a careful consideration must be made of when events are exactly triggered, especially during the initialization of a GTU. The RoadSampler now has to perform quite a few tricks to make sure all relevant information is available at the right time of calling the functionality in its parent Sampler.

[averbraeck]

I would suggest adding a page in the documentation (which is now part of the docs-folder here on github) that defines the exact contract for which events are fired when, plus stating what each event is meant for and which object fires it. It's a kind of reference page, but pretty useful to check if the right events are fired at the right time.

[WJSchakel]

I agree. This broadens this issue a bit, but let's get things right. An overview of all events is now in the doc folder (see link below). It is not yet part of the documentation menu structure. This can be published in final form later. https://opentrafficsim.org/manual/02-model-structure/events/

[WJSchakel]

Some observations:

• In Network there are many events. Each has two forms where one has ANIMATION_ at the start of the name, e.g. GTU_ADD_EVENT and ANIMATION_GTU_ADD_EVENT. @averbraeck do you know why this is? It seems we could merge all these events.
• There are many events not used. I suggest removal of the following. From Network: (ANIMATION_)INVISIBLE_OBJECT_ADD_EVENT), (ANIMATION_)INVISIBLE_OBJECT_REMOVE_EVENT, (ANIMATION_)ROUTE_ADD_EVENT, (ANIMATION_)ROUTE_REMOVE_EVENT. From LaneBasedGtu: LANEBASED_INIT_EVENT and LANEBASED_DESTROY_EVENT (what do these really add over Gtu.INIT_EVENT and Gtu.DESTROY_EVENT?). From AbstractPlot: GRAPH_ADD_EVENT and GRAPH_REMOVE_EVENT.
• I have reservations on the class DirectionalOccupancySensor and its events DIRECTIONAL_OCCUPANCY_SENSOR_TRIGGER_ENTRY_EVENT and DIRECTIONAL_OCCUPANCY_SENSOR_ TRIGGER_EXIT_EVENT as we removed directionality. These events are currently not used (their non-directional counterparts are). Perhaps the whole class should be removed.

[WJSchakel]

The main cause for this issue is the relation between RoadSampler and the various GTU events, particularly at the start of a GTU's session. The following is a summary of the procedure at the start of a GTU.

LaneBasedGtu constructor
    Gtu constructor
        PerceivableContext.addGtu()
            Network.ADD_GTU_EVENT
LaneBasedGtu.init()
    1 micrometer operational plan is set
    LaneBasedGtu.LANEBASED_INIT_EVENT // proposed above to be removed
    Lane.addGtu()
        Lane.GTU_ADD_EVENT
        OtsLink.addGTU()
            Link.GTU_ADD_EVENT
    Gtu.init()
        Strategical and tactical planner set
        Gtu.INIT_EVENT
        LaneBasedGtu.move()
            Gtu.move()
                determine operational plan
                schedule next move event
                Gtu.MOVE_EVENT
            LaneBasedGtu.LANEBASED_MOVE_EVENT

This is a bit of a mess, the code is also quite chaotic. But I do not think parts of this complexity will cancel each other out; this is a string of chicken-egg problems. Therefore an initially proposed contract for events could be:

1. ADD events make no guarantee whatsoever as to the state of the added object, other than it having an id. They can be used to register listeners with the object, and to allocate resources for the object (e.g. Map.put(gtuId, new ArrayList<Speed>());.

2. INIT events make no guarantee about the state of the initialized object, but do commit to be thrown as late as possible during object initialization, though always before any other events of an object. (In the procedure above, Gtu.INIT_EVENT cannot be later in the procedure as a Gtu.MOVE_EVENT will be thrown. Note that LaneBasedGtu.move() inserts itself. But from the perspective of Gtu, the Gtu.move() method is invoked from Gtu.init(), and a Gtu.MOVE_EVENT is expected. The subclass LaneBasedGtu has to respect this behavior, and indeed it does by calling super.move() = Gtu.move() itself.)

3. MOVE and events of all other types must occur with the object in a complete state.

This contract would keep the above procedure. Listeners (e.g. RoadSampler, TrafficLightSensor) will have to adhere to this contract. To stimulate following this contract, the ADD events should have a payload with the added object id, and perhaps other data of the object it is added to (e.g. new GTU count on a lane). But these events should not have more information about the added object. In fact, Network.ADD_GTU_EVENT, Lane.GTU_ADD_EVENT and Link.GTU_ADD_EVENT already adhere to this contract. It is actually e.g. RoadSampler (and Sampler) that need a critical look at their own procedure. For RoadSampler the processGtuAddEvent() method performs an immediate processGtuMoveEvent(). Instead it could:

• As a reaction to Lane.GTU_ADD_EVENT, listen to LaneBasedGtu.LANEBASED_MOVE_EVENT.
• As a reaction to LaneBasedGtu.LANEBASED_MOVE_EVENT; if first encounter, invoke processGtuAddEvent(). Next, always invoke processGtuMoveEvent().

If in a similar fashion we update StochasticDistractionModel, the whole category of INIT events could be dropped, finally resulting in the contract:

1. ADD events make no guarantee whatsoever as to the state of the added object, other than it having an id. They can be used to register listeners with the object, and to allocate resources for the object (e.g. Map.put(gtuId, new ArrayList<Speed>());.
2. MOVE and events of all other types must occur with the object in a complete state.

[averbraeck]

The ANIMATION_ events were added to pass a pointer to the object itself, rather than to its id. This means that, e.g., DefaultAnimationFactoty in ots-draw can subscribe to the ANIMATION_ events and do:

if (event.getType().equals(Network.ANIMATION_GTU_ADD_EVENT))
{
    // schedule the addition of the GTU to prevent it from not having an operational plan
    LaneBasedGtu gtu = (LaneBasedGtu) event.getContent();
    this.simulator.scheduleEventNow(this, this, "animateGTU", new Object[] {gtu});
}

The sending of the id rather than the object in the other event is important, for instance, when sending information over the network, e.g., in case when the EventProducer is an RmiEventProducer. In that case, sending a Gtu over the network could result in sending an entire model over the network (Gtu has a pointer to Network, which has a pointer to Simulator, which has a pointer to Model). To avoid this, the id is notmally sent.

[averbraeck]

In theory, the ANIMATION events could be removed, and just retain the events that do send an Id, if we pass the Network to classes such as DefaultAnimationFactory. When they have a pointer to the Network, they can look up the Gtu, Node or Lane, etc. on the basis of its id.

Note, though, that this then also should hold for detectors, conflict areas, and any other object that needs to be drawn. Every object that needs to be drawn then needs to reside in Network... The ANIMATION_ events tried to avoid us forcing to do this.

[averbraeck]

Removing events like adding or removing objects from the model, adding or removing dynamic graphs, etc. means that animation and user interfaces will not be able to update their status anymore based on changes in the model. Only static models can then be created. This also prevents making editors for infrastructure, which would be heavily dependent on these events. I would advise against removing of (most of) these events.

[WJSchakel]

DefaultAnimationFactory already has a simulator, so the step towards having a OtsNetwork seems small to me. In fact, the constructor already has that as an argument. In general, it does not have to be an OtsNetwork where objects may be found. There just needs to be some specific context where objects of a specific type register themselves, and where listeners of that type can retrieve them from. Right now we are sort of abusing events for two purposes:

1. Sending information over the network for interaction with remote tools.
2. Triggering local interactions between simulation components.

I'm not sure what to do here. I see your points, but this huge list of semantically duplicate events is bothering me. It is a lot of code clutter, and confusing to me, let alone other users of OTS.

[WJSchakel]

Removing events like adding or removing objects from the model, adding or removing dynamic graphs, etc. means that animation and user interfaces will not be able to update their status anymore based on changes in the model. Only static models can then be created. This also prevents making editors for infrastructure, which would be heavily dependent on these events. I would advise against removing of (most of) these events.

I can see a case for ROUTE_ADD, as graphs could be along routes, or statistics could be based on routes. But regarding INIVISBLE_OBJECT_ADD, I'm not sure what the purpose is, given that we have OBJECT_ADD.

As to AbstractPlot.GRAPH_ADD_EVENT and AbstractPlot.GRAPH_REMOVE_EVENT, these are currently not even thrown. But given what you say I can see a future for them.

What about LaneBasedGtu.LANEBASED_INIT_EVENT, Gtu.INIT_EVENT (i.e. the whole init phase) and LaneBasedGtu.LANEBASED_DESTROY_EVENT?

[averbraeck]

1. Sending information over the network for interaction with remote tools.
2. Triggering local interactions between simulation components.

Well, this is what Events are meant for... Both purpose 1 and purpose 2. Purpose 1 is an extension of Purpose 2 in case the (simulation) components that need to inform each other are remote.

Note that Events are only used when the Listeners can change, and as a result these Listeners are unknown to the caller. When you know which objects to inform about a change, inform them with a method call. When these objects are dynamic, use events. Network listeners are usually by default dynamic, since sometimes the listener is started first, and sometimes the producer is started first. So is animation: it can be switched off; simulations can run without animation; or there can be multiple animations for the same model with a different viewpoint.

[averbraeck]

For now, I would be in favor of removing the ANIMATION_ events. Classes like AnimationFactory can easily look up the corresponding classes in the Network. When it is difficult to find an object that is not registered in the Network, we can extend the payload of the event as a solution.

So, yes, remove the confusing events. But keep events in general that indicate the addition or removal of model objects and user interface objects.

[averbraeck]

What about LaneBasedGtu.LANEBASED_INIT_EVENT, Gtu.INIT_EVENT (i.e. the whole init phase) and LaneBasedGtu.LANEBASED_DESTROY_EVENT?

I struggled with those when implementing, and chose for more events. But indeed, it is totally unclear. I would say, only one type of INIT event and one type of DESTROY event. It should be thrown on construction and destruction by the most basic class (so in ots-core). When you want to know whether it is LaneBased or not in the notify method, retrieve the Gtu from the Network, based on its id, do an instanceof, and act on what you get from the instanceof test.

[WJSchakel]

Agreed. I am wondering what the purpose of INIT is though. By some ADD from the context, a listener becomes aware of an object. What is the logic of this listener? "Great, now I can listen to its INIT event to find out when it first appears in simulation... wait I already know that from the ADD that just happened."

The reverse is not true for DESTROY, as a GTU could for example in principle switch between networks. An animator of GTU's for one network, might not need to destroy the GTU animation on Network.GTU_REMOVE_EVENT, but rather hide it for the time being. In case of the DESTROY event, then the GTU is really at its end.

But regarding Gtu.INIT_EVENT, would it perhaps be some payload that is valuable?

[averbraeck]

... INIVISBLE_OBJECT_ADD, I'm not sure what the purpose is, given that we have OBJECT_ADD.

ObjectInterface and InvisibleObjectInterface are two different things. Both are stored in the Network, and have their own maps, and add and remove methods.

• ObjectInterface is meant for a generic object that can be placed in the model. This could be implemented for a traffic light, a road sign, or an obstacle. (from the Javadoc).
• InvisibleObjectInterface is for objects that live in a Network, but cannot be drawn and which do not have a specific location. These objects do have a name and need to be cloned when the Network is cloned. Example: TrafficLightController. (from the Javadoc).

Since we do not clone networks anymore, it might be unnecessary to store InvisibleObjects in the Network. But for now, let's keep them. Again, for listing them in a UI, for editors, and at some point, for storing the state of a model to disk, having these objects in our 'what's in the simulation model' class can be quite useful.

[averbraeck]

But regarding Gtu.INIT_EVENT, would it perhaps be some payload that is valuable?

@WJSchakel : You actually use this event in the StochasticDistractionModel. Here, you explain why you need another event in addition to the GTU_ADD_EVENT: (literal quote from Javadoc): The GTU is not initialized yet, so we can't obtain the tactical planner.

[WJSchakel]

• ObjectInterface is meant for a generic object that can be placed in the model. This could be implemented for a traffic light, a road sign, or an obstacle. (from the Javadoc).
• InvisibleObjectInterface is for objects that live in a Network, but cannot be drawn and which do not have a specific location. These objects do have a name and need to be cloned when the Network is cloned. Example: TrafficLightController. (from the Javadoc).

Ahhh, well then 'InvisibleObject' is a terrible name if you ask me. In a non-programming context I would say these are abstract objects, but that is even more confusing in a programming context. Can't think of a better name right now.

[averbraeck]

In general, when removing the ANIMATION_ events, let's be sure that anyone asking the network for the object belonging to the id that is in the non-animation event, can be sure that it is contained in the correct map Network class, and can immediately be retrieved. I think that is the case right now, but it does not hurt to check (and write a unit test for it).

[WJSchakel]

But regarding Gtu.INIT_EVENT, would it perhaps be some payload that is valuable?

@WJSchakel : You actually use this event in the StochasticDistractionModel. Here, you explain why you need another event in addition to the GTU_ADD_EVENT: (literal quote from Javadoc): The GTU is not initialized yet, so we can't obtain the tactical planner.

Yes, but I'm also critical on my own coding there. It could work without an INIT. It could simply listen to a MOVE and do the same on the first move, and then stop listening to MOVE. I mentioned this somewhere above in this, admittedly, rather big discussion.

[WJSchakel]

I mentioned it (implicitly) here. Should have been explicit that that change in StochasticDistractionModel can be done.

If in a similar fashion we update StochasticDistractionModel, the whole category of INIT events could be dropped, finally resulting in the contract:

1. ADD events make no guarantee whatsoever as to the state of the added object, other than it having an id. They can be used to register listeners with the object, and to allocate resources for the object (e.g. Map.put(gtuId, new ArrayList<Speed>());.
2. MOVE and events of all other types must occur with the object in a complete state.

[averbraeck]

I really like this. We often forget we can drop listening to events after the first 'hit'.

[averbraeck]

Can't think of a better name right now.

Haha. That was exactly my problem at the time when I implemented it. Abstract has a very different meaning, though, in a Java programming context. Names could be:

• ConceptualObject
• ImaginaryObject
• VolatileObject
• NonLocatedObject
• UnlocatedObject

I like ConceptualObject and NonLocatedObject best.

[WJSchakel]

With the use of wikipedia, I get to SpiritObject... NonPhysical is in a sense incorrect, our objects are within time. https://en.wikipedia.org/wiki/Non-physical_entity

Conceptual has some overlap with Abstract I think. So I would go for NonLocatedObject. I will include this change.

[averbraeck]

Shall we then also change ObjectInterface into LocatedObject? Like with InvisbleObjectInterface, I also felt uncomfortable with ObjectInterface...

[WJSchakel]

Yes, will do.

[WJSchakel]

The RoadSampler and Sampler classes have been cleaned-up.

• In Sampler, processGtuAddEventWithMove has been split in to processGtuAddEvent and processGtuMoveEvent. RoadSampler uses these in a more fine-grained manner.
• Removed repeated get()/contains() calls on maps with bookkeeping.
• RoadSampler.notify() is now much clearer. For each event the code has three phases: payload handling, the core action, bookkeeping. Bookkeeping is not required for MOVE. For ADD and REMOVE this step is clearly separated between interval-based and move-based sampling.
• Variable names and method names have been made more self-evident.

Sampling works both interval-based and move-based, and both with or without instantaneous lane changes. Move-based and without instantaneous lane changes there is a peculiarity. At the start of the lane change an ADD occurs on the target lane, resulting in a snapshot on that lane. During the lane change the MOVE is only triggered on the reference lane, which is the origin lane during the first halve of the lane change. At this point I'm leaving this, as sampling will have to be adapted in the future to space-based movement anyway.

[WJSchakel]

INIT events have been removed. Documentation of events (events.md) needs some more description. The current table is more for development such that we can have a critical look at the events. Once that is done, we can describe what the events are meant for. The 'Listeners' column can be dropped from the table. But this is no longer really part of this issue.