hooks to make PhET-iO API calls on 'step'

[pixelzoom]

From 7/18/19 Gas Properties design meeting notes in phetsims/gas-properties#30 (comment):

• Are there hooks to make API calls on step?

The use case is to allow the client to get an array of particle speeds each time the model is stepped.

The answer is "not currently supported". There are some ideas in the Slack discussion below, but no final decision. @kathy-phet @ariel-phet @arouinfar FYI, prioritization needed.

Slack discussion

Chris Malley 3:14 PM
Does PhET-iO provide clients with a way to optionally call a function on each call to Sim stepSimulation? The use case is in Gas Properties, where the design team would like to provide an API function that allows the client to get an array of particle speeds each time the model is stepped.

Michael Kauzmann:house_with_garden: 3:15 PM
@jessegreenberg @samreid and I were just looking at that! We noticed that stepSimulationAction is an Action, so we can't add listeners to it for PhET-iO
Not sure if @samreid or @jessegreenberg made an issue.

Mainly we noted that phetsims/axon#241 wasn't complete, because ActionIO has an addListener function which is buggy.

Chris Malley 3:17 PM
Listen to frameStartedEmitter or frameEndedEmitter?

Michael Kauzmann:house_with_garden: 3:18 PM
Yes perhaps that is the solution, though they aren't currently instrumented. That seems like adding unneeded complexity. That said I think that may be the best/easiest solution.

Chris Malley 3:18 PM
It also provides the flexibility to be notified before or after the step has occurred.

Michael Kauzmann:house_with_garden: 3:19 PM
So you think that perhaps instrumenting both is the way to go?

Chris Malley 3:19 PM
This is one of 3 “Ask PhET-iO Team” action items that I have from last week’s Gas Properties PhET-iO meeting. Perhaps I should create an issue for each of them? See phetsims/gas-properties#30 (comment)
The other alternative would be a sim-specific {Emitter} particleSystemChanged that fires each time the particle system has changed. But I’m hesitant to add any more overhead that occurs in step.

Michael Kauzmann:house_with_garden: 3:21 PM
Sounds excellent. Also it would be good to note that since phet-io listeners are inherently async, perhaps only the frame ended emitter should be instrumented, because likely the frame will complete before the thread executes anything from the wrapper frame.

[samreid]

There is discussion and a proposal in phetsims/axon#222 (comment) about how to deal with Action and listeners. In this case, it would mean instrumenting Sim.frameEndedEmitter. @zepumph and @pixelzoom sound OK?

[pixelzoom]

Sim.frameEndedEmitter sounds fine for hooks at the end of a frame. You'll need to ask the designers if they think there's a need for hooks at the start of a frame.

[samreid]

@zepumph pointed out in another thread that this method is asynchronous, and it is unnecessary to have both started and ended, since they would likely be called at the same time anyways.

[kathy-phet]

I don't know what happens on a frame well enough to know whether we would need control over the start or the end of the frame. I think what would be important is that we don't get model values that are mid-way calculated .. e.g. PV before being updated and NT from after being updates. We need them to be either all calculated or all before the calculation update.

[zepumph]

My vote would be for instrumenting the frameEndedEmitter, not both.

[zepumph]

We need them to be either all calculated or all before the calculation update.

Because of the asynchronous nature of phet-io when communicating from the sim frame to the wrapper frame, all calculations would be done in the sim for a given frame before the wrapper get's any messages for that frame.

[pixelzoom]

I agree with @kathy-phet's comment in #570 (comment), but that pertains to a specific case, Graphing Quadratics. The question about whether both pre and post callbacks are needed is a general question. If the answer is "no", that's fine. I just want to make sure we're considering this generally, not doing something specific for Graphing Quadratics.

[kathy-phet]

I think Chris means Gas Properties.

[pixelzoom]

Sorry, yeah, Gas Properties (the other "G" sim :)

[samreid]

This is pending discussion in https://github.com/phetsims/phet-io/issues/1543, on hold until we discuss there.

[zepumph]

@samreid and I think we will just make this an Emitter. It makes sense to convert the stepSimulationAction to be an Emitter so that it can be listened to. Over to @samreid for implementation.

[samreid]

Some questions for @zepumph before we proceed:

1. stepSimulationAction is currently type {Action}. All 24 of our phetioPlayback: true are currently {Action} and none of them are {Emitter}. Does the playback engine support Emitter?
2. Previously in this issue we discussed instrumenting the frameEndedEmitter but the prior comment is about stepSimulationAction, what is your recommendation?

Please reassign to me after answering.

[zepumph]

stepSimulationAction is currently type {Action}. All 24 of our phetioPlayback: true are currently {Action} and none of them are {Emitter}. Does the playback engine support Emitter?

Yes I believe so. Currently there are no Emitters that are phetioPlayback:true, but all the wrapper-side cares about is the "emitted" event, which Actions and Emitters send (in handlePlaybackEvent.js:

      // handle playback Actions
      else if ( event.name === 'emitted' ) {
        const orderedArgs = event.metadata.dataKeys.map( argumentName => event.data[ argumentName ] );
        totalInvokes++;
        phetioClient.invoke( event.phetioID, 'execute', orderedArgs, completedInvoke );
      }

Previously in this issue we discussed instrumenting the frameEndedEmitter but the prior comment is about stepSimulationAction, what is your recommendation?

Yes sorry. Any one of these, no preference. frameEndedEmitter is great.

[pixelzoom]

"Each time the model is stepped" is not the same as "the end of every frame". Sim.frameEndedEmitter continues to emit when the model is not being stepped.

[samreid]

@pixelzoom can you please assign to the designer(s) for this sim for clarification?

[pixelzoom]

Since this issue is a breakout from meeting notes in phetsims/gas-properties#30 (comment), assigning to @arouinfar.

[arouinfar]

From a design perspective, it doesn't seem particularly important to have the emitter fire while the sim is paused, but it also doesn't seem harmful. While the sim is playing, does frameEndedEmitter align with the model being stepped? If so, that would fulfill the original request. I'm not sure of the pros/cons of using frame vs. step and which is preferable in a more general case. If you want more designer feedback, please add this issue to the phet-io meeting agenda.

[samreid]

There is no standard interface for pausing and running models, so something would have to be done on a sim-by-sim basis for this. I'm not sure it is worth the effort. If the client only wants updates when the model is running, they could add a check that says

client.invoke('frameEndedEmitter','addListener',[()=>{
    
// check if the model is running.  If it is, then collect the data
client.invoke('model.isRunningProperty','getValue',[],(returnValue)=>{

  // Only access the particle speeds if the model is running
  returnValue && client.invoke('model','getParticleSpeeds',[],(particleSpeeds)=>{
  // etc.

});
});

}]);

PhET-iO used to have a way to pass an API call "schema" to the core side to evaluate at a specific time synchronously, but we dropped support for that and now have to use the asynchronous method from above.

[pixelzoom]

I'm still not sure what the requirements are here, so I don't know what additional feedback to give. The closest I can find to a requirement is what @kathy-phet alluded to in #570 (comment):

I think what would be important is that we don't get model values that are mid-way calculated .. e.g. PV before being updated and NT from after being updates. We need them to be either all calculated or all before the calculation update.

No idea what PV and NT mean. And I would think that view values would be as important as model values. But this sounds like a desire to have a way to get notification that all values have finished changing, and you can safely check values without getting any intermediate values. If I interpret that correctly, then frameEndedEmitter gives you that, with the inconvenience of being notified even when nothing could have possibly changed in the sim.

[arouinfar]

I don't think an asynchronous conversation is going to be very productive here, so I've added this issue to the agenda for next Thursday's phet-io meeting.

[zepumph]

We discussed this at phet-io meeting today, and designers and devs feel like the way @samreid outlined in #570 (comment) will work for this use case, and if, in the future, the asynchronous behavior is not acceptable, then we can investigate adding support for running commands inside the sim frame.

@pixelzoom how does this sound to you? Anything else here?

[pixelzoom]

I've never been clear on the requirements, and this issue was opened at the request of designers. So if designers are OK with what @samreid proposed in #570 (comment), then that sounds fine to me.

Should an example be added to client-facing documentation?

[zepumph]

Maybe it would be nice to start a "FAQ" section of the wrapper index, that can expand for some use cases that devs have as they do. Part of me feels like this goes beyond the scope of our client facing doc right now, and I'm tentative to add to our current wrapper index before it gets a face lift.

[zepumph]

I think we can close this issue. There is no good place to add client facing documentation at this time. If we feel like this is an important feature for a sim, then it can be added its client guide.

Closing