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

ev: issues with ChargingHandler.notifyMobsimAfterSimStep #3545

Open
sebhoerl opened this issue Nov 6, 2024 · 5 comments
Open

ev: issues with ChargingHandler.notifyMobsimAfterSimStep #3545

sebhoerl opened this issue Nov 6, 2024 · 5 comments

Comments

@sebhoerl
Copy link
Contributor

sebhoerl commented Nov 6, 2024

I have a use case where I react on ChargingStartEvents using a simple event handler, and it is critical that timing is right. In particular, I collect various events in time step T and then process them in a MobsimEngine.doSimStep in time step T + 1. This way, we are sure to handle everything that happened in T at once and not have events arriving in parallel to the logic implemented in doSimStep. This is a common pattern that we use in various contribs (for instance, sharing).

Now for ChargingStartEvent this mechanism breaks, because the events are generated over several stations inside ChargingHandler.notifyMobsimAfterSimStep:

  • The ChargingStartEvent is generated at the end of T in ChargingHandler.notifyMobsimAfterSimStep
  • However, this happens after the event handlers have been instructed to synchronize on time step T
  • So the event (which has time=T) still may arrive when everything is in the state of time step T, then my event collection logic still adds the event to the list of events collected in T
  • But it may happen that publication of the event takes slightly longer, so it will arrive after I have flushed my collected events from T and it will be added to the collected events of T + 1
  • In consequence, everything else in my business logic happens one time step later, i.e. the charging event will only be processed in T + 2 in some rare cases, but usually in T + 1 as expected.

My conclusion from that would be:

  • We should never write events inside notifyMobsimAfterSimStep. Somehow I even think that I noticed during development of the sharing contrib that this used to be forbidden (enforced?) in some way.
  • In the particular case of the ChargingHandler, I'd like to transform this class into a MobsimEngine and generate the events in a doSimStep. Or has this been tried already and there have been issues? @michalmac
@kainagel
Copy link
Member

kainagel commented Nov 6, 2024

We had the same problem (with the charging handler, for the same reason) some other time. I would agree that moving it into the mobsim reduces the problem, since in the mobsim we have "physical" sequences of activities and in consequence should have correspondingly sequenced events. However, for this we need that "patches" of physics are on the same thread/CPU (i.e. both the vehicle and the charging station to which it connects). I am a big fan of such "physical patches", but pieces of matsim infrastructure go into a different direction. @paulheinr , @Janekdererste , @rakow , @mrieser: Maybe you want to take not of this issue for other parallel implementations we are working on?

@michalmac
Copy link
Member

IIRC I was thinking about moving the whole "EV simulation" inside mobsim. It felt very natural. However, the biggest hurdle was the energy consumption part, where currently we listen to traffic/link entered/left events. Simulating charging inside mobsim and discharging outside it seems a bit fragile (with a high likelihood of running into some race conditions). As far as I remember, my conclusion was that it would be best to somehow plug in the energy discharging logic directly into QSim (i.e. not via events), so whenever a vehicle moves, its SoC gets immediately updated.

@michalmac
Copy link
Member

I also do not like the feedback loop from events to back to simulation. For instance, vehicle events --> energy discharging events --> change in SOC which may impact Mobsim at this or next step (e.g. vehicle can or cannot travel the link because of negative SOC).

@sebhoerl
Copy link
Contributor Author

sebhoerl commented Nov 6, 2024

FYI, I now have the following pattern which should work robustly in this kind of situations:

class MyMobsimEngine implements MobsimEngine, ChargingStartEventHandler, MobsimScopeEventHandler {
  // Event collection part
  
  private final List<ChargingStartEvent> chargingStartEvents = new LinkedList<>();

  @Override
  public void handleEvent(ChargingStartEvent event) {
    synchronized (chargingStartEvents) {
      chargingStartEvents.add(event);
    }
  }

  // Event processing part
  
  @Override
  public void doSimStep(double time) {
    processPersonDepartureEvents(time);
  }

  private void processPersonDepartureEvents(time) {
    synchronized (chargingStartEvents) {
      var iterator = chargingStartEvents.iterator();

      while (iterator.hasNext()) {
        ChargingStartEvent event = iterator.next();
        
        if (event.getTime() < time - 1.0) {
          iterator.remove();
          // do something with the event
        }
      }
    }
  }
}

The if (event.getTime() < time) is important in case the events of the previous or current iteration can arrive before and after calling processPersonDepartureEvents. And -1.0 is necessary for events that are thrown in an afterMobsim listener.

@jfbischoff
Copy link
Collaborator

IIRC I was thinking about moving the whole "EV simulation" inside mobsim. It felt very natural. However, the biggest hurdle was the energy consumption part, where currently we listen to traffic/link entered/left events. Simulating charging inside mobsim and discharging outside it seems a bit fragile (with a high likelihood of running into some race conditions). As far as I remember, my conclusion was that it would be best to somehow plug in the energy discharging logic directly into QSim (i.e. not via events), so whenever a vehicle moves, its SoC gets immediately updated.

I remenber our discussions about that.
Keeping discharging out of the QSim has some advantages as well though, the handling is far more intuitive in a MATSim sense (we don't really care what's going on while a vehicle is on a link), and handling discharging every timestep is probably more intense in computational terms.

Apart from historic comments, I have no strong opinions on design changes 😄

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 a pull request may close this issue.

4 participants