PhET-iO instrumentation

[chrisklus]

We are planning on having a design meeting for PhET-iO instrumentation of this sim while it's still fresh for @jbphet and me.

[chrisklus]

Whoops!

[ariel-phet]

@chrisklus is going to implement with basic tandems and then request a design meeting (current target sept 12 design meeting)

[chrisklus]

@ariel-phet when I said two weeks during our discussion I was intending for 09/05/19 design meeting, if that's a possibility. A dev test has been opened, so if nothing crazy comes from that then I'll be able to do initial tandem passing before Thursday next week.

[chrisklus]

Before instrumenting

• Create a "PhET-iO Instrumentation" issue in the simulation repository. Copy this checklist to the issue description (top issue comment) for tracking. Link back to this checklist via /blob/<SHA>/ so that the specific guide you used is preserved.
• Publish a pre-instrumentation dev release. This can be useful for identifying whether bugs or memory issues have been introduced during instrumentation, or were pre-existing. This also creates a benchmark to reference against for memory-leaks, sim size, performance, etc. Document the dev release in the sim's phet-io github issue.

• pre-instrumentation dev version: https://phet-dev.colorado.edu/html/energy-forms-and-changes/1.2.0-dev.2/phet/energy-forms-and-changes_en_phet.html

• Understand the goal. Read through The PhET-iO Website Dev Guide including the link to Graphing Quadratics 1.1. Visit all of the linked wrappers and docs. Test each wrapper, investigate, report bugs, ask questions!
• Understand the complete PhET-iO feature set and api. In general it can be thought of as 4 items:
  • The ability to customize PhET-iO elements within the simulation. Each PhET-iO element corresponds to a PhetioObject instantiated in the sim along with its associated TypeIO. For example, SCENERY/Node extends PhetioObject and its default TypeIO is NodeIO.
  • The ability to get and set the state of the simulation.
  • The data stream of the simulation. Some of these events can in turn be used to power event driven playback.
  • The interface for communicating with a PhET-iO instrumented sim. In general this won't need to be adjusted or added to while instrumenting a simulation, but it can be good to familiarize yourself with basic wrapper making code to understand how PhET-iO is meant to be used. To get started see the template example in the wrapper index.
• Schedule a PhET-iO design meeting for the simulation to identify what should be customizable/interoperable/data stream and track it in an issue. For example, see how-to-design-phet-io-features-for-a-simulation.md. Think about how a researcher or 3rd party may wish to configure the simulation or collect data from it, and make sure that is supported by the instrumentation. For example, some simulations will need custom higher-level events (such as
  whether the user created a parallel circuit), for events that are useful, easy to compute in simulation code and difficult to compute in wrapper code. Or a simulation may need to be configurable in a way that is not already supported by the instrumentation you have already completed. These features should be determined in the PhET-iO design meeting.
• Typically it is best if the responsible developer for the simulation is available to perform the PhET-iO instrumentation. They have important insight into the structure, history, trade-offs and other important details of the simulation implementation that will facilitate the instrumentation. If the responsible developer is not available for instrumentation, it would
  be nice if they are available for consultation or support during instrumentation.

Code Review

A high-quality code review will make instrumentation easier, promote long term maintainability for the simulation, and protect the simulation from a volatile API. If the simulation is already in good shape, the review will not take too long. If the simulation is not in good shape, then it needs your help.

• Read through the open issues and be aware of any outstanding problems, future work, etc.
• If there is a branch with significant effort, consider merging it before instrumentation.
• Complete any planned refactorings.
• Address TODOs in the code
• Bring the sim up to standards.
• If there are sim components that can be exchanged to use newer common code ones, do so. Consulting phet design patterns may be helpful.

Instrumentation

Now that the simulation is in good shape and the PhET-iO design meeting is complete, we are ready to instrument the simulation. Follow the checklist below, and if you have questions you can review Faraday's Law or Graphing Quadratics and their PhET-iO instrumentations, or reach out to teammates who may have come this way before.

Initial Setup

• Add 'phet-io' as a supportedBrand in the sim's package.json. A script on Bayes will automatically add the simulation to the list of phet-io simulations. This will make it possible to use phetmarks to launch wrappers for testing. This also will add it to continuous fuzz testing.
• Run grunt update in the sim repo. This will generate the needed phet-io api preload files to run in phet-io brand. Note that while validating the api, these preloads will need to be kept in sync with the current state of the repo. This may be more trouble than it's worth while iterating. You can use ?phetioValidateAPI=false to turn off this validation.
• Import Tandem to main.js, see faradays-law-main.js for an example
• Pass tandems to each screen using tandem.createTandem(...)
• The PhET-iO Studio wrapper will serve as the foundational approach to understand, test, and implement a phet-io instrumentation. It displays a list of all "PhET-iO" elements and has controls to interoperate with them. Please note that studio does not demonstrate the entire suite of phet-io features, and thorough testing of all wrapper suite wrappers is vital to understanding the intricacies of the instrumentation process and goals. You can remove the ?phetioValidateTandems query string to temporarily get past "Tandem was required but not supplied" issues.

Visit Objects that Should be Instrumented

Consult the PhET-iO design issue to see what features the sim should support. See
PhetioObject.js for the
supported PhetioObject options. Not every node in the hierarchy must be instrumented, but every leaf is instrumented. For example the view is rarely instrumented.

• Recursively pass tandems and other PhetioObject options into objects that should be instrumented. Do not instrument objects that are "implementation details" and do not over-instrument. The goal is a to design an API that balances the power of a broad feature set while still being maintainable.
• Instrument user interface components such as Checkbox, HSlider, etc.
• Instrument model components such as AXON/Property that are critical to the save state or operation of the sim. This does not necessarily include "implementation details" that should be hidden from the public API; again a design meeting may be needed here. (Note that some Property sub-classes utilize options specific for use with PhET-IO, units
  in NumberProperty for example, and should be passed where appropriate.)
• Instrument all of the features identified in the simulation PhET-iO design issue.

Creating and Naming Tandems

Well-designed tandem names are important. Once the PhET-iO simulation is published, the API becomes public and therefore difficult to change. Sometimes PhET-iO design meetings can also help come up tandem names. NOTE: "Tandem" is a PhET internal name, publicly to clients the full strings are known as "phetioIDs" referring to PhET-iO elements.

• Tandems should be named as we wish clients to see them, and for long-term stability. For maintainability, local vars should be renamed to match tandem names.
• Screen tandems should end with a Screen suffix.
• Property instances should have Property suffix.
• The screen's model and view should be named model and view.
• When adding tandem args to constructors, please follow the following heuristic regarding required vs optional tandem args from Should tandem be in options or required param? joist#489 (comment):
  • Use @param tandem for constructors that don't have an options parameter. This typically includes top-level model and view types that are specific to the sim.
  • Use tandem in options object: Tandem.required for constructors that already have an options parameter. This default can be helpful for identifying cases where you have neglected to pass a tandem in (because Tandem.required will error loudly if validating tandems).
• Use createGroupTandem for arrays, dynamic instances, or otherwise numbered tandems. See usages for examples.
• Use tandems statically where necessary (outside of constructors). For instance, static objects that are not created from the main sequence. See BeersLawSolution.js for an example.
• All tandems for a sim should exist under a screen, or in the global section, see Tandem.globalTandem.

Feature Support

• In addition to passing a Tandem instance, each PhetioObject should be provided a TypeIO. The TypeIO for a PhetioObject indicates the public api for that PhetioObject instance. Most instrumented common code Types already have a TypeIO provided as a default option for phetioType.
• Where appropriate, create or instrument Property instances to make it possible to get/set a value, so value changes will appear on the data stream and so the item can be stored and restored in save/load. This is preferred to creating a new TypeIO and implementing get/set within that type.
• Each TypeIO has a validator static that can be used to validate the type. When instrumenting a Property, Emitter, or other type that validates parameters in which that instance provided valueType for validation, in most cases the TypeIO's validator will be redundant to the valueType field. If this is the case, the valueType can and should be removed to keep the code simpler and more maintainable.
• If necessary, instrument common code components that are not yet instrumented. You can check if something is instrumented by checking whether it extends PhetioObject and whether it supplies any PhetioObject options. To instrument a new common code component, you may need to add instrumented Property or Emitter elements by composition, or subclass PhetioObject.
  Run phetmarks=>aqua=>Test Sims(Fast Build) with PhET-iO checked. This will help catch any simulations using the component you just instrumented.
• Add tandem: Tandem.required or tandem: Tandem.optional to the options accordingly. Here are some conventions to guide this decision:
  • Most UI components are Tandem.required
  • Even if a common code component that your subtype extends is a certain option, it is safest to set this tandem constraint at the subtype level in sim specific code too.
  • The safest convention for required tandems is to add tandem: Tandem.required to default options wherever you intend to pass tandem via options.
  • For more information/context see What subtypes need to extend Tandem.required? graphing-quadratics#64 (comment)
• Note Node already extends PhetioObject--its PhetioObject options should be provided to the constructor or mutate but not both.
• Use the phetioPrintMissingTandems flag if you want to collect a list of all required, optional, and uninstrumented common code classes instead of erroring out on the first missing tandem. Each occurrence is numbered to give a better idea of how many the sim has to do.
• Transient state should not be saved. For instance, whether a button is highlighted from mouseover, or whether the About dialog is showing should not be part of the save state of the simulation.
• Run the simulation with ?phetioValidateTandems to see if you missed anything that should be instrumented.
• Add the simulation to perennial/data/testable-phet-io-validated so CT will test with ?phetioValidateTandems
• Use the following conventions regarding phetioDocumentation option:
  • Documentation strings do not have to be in complete sentences, i.e. "The location of the center of the bar magnet in view coordinates" seems complete even though it is just a subject.
  • Following up on this, the above example does not need (and shouldn't have) a period because it isn't a sentence.
  • We want phetioDocumentation that is client facing to start with a capital letter.
  • We allow HTML in our documentation strings to be rendered by the wrapper.
  • There is no need to restrict any characters, if chars break the HTML, it should be manually verified in Studio before publication.
  • For now we there isn't validation, only manual inspection as you instrument.
  • see https://github.com/phetsims/phet-io/issues/1334#issuecomment-405368103 for context
• Port vibe audio (if any) to tambo. See Rewrite Vibe to use Tambo vibe#33 and note that PhET-iO query parameters support tambo but not vibe.
• Avoid instrumenting values in DEFAULT_OPTIONS or at least be very careful about how it is done, see the concerns mentioned in https://github.com/phetsims/phet-io/issues/1179
• For UI components, consider whether to link to the underlying Property via addLinkedElement

Create new TypeIOs

If necessary, create new TypeIOs to support desired feature set. Generally we don't want to be locked in to coupling
TypeIOs to sim types. Instead, we decided that we want the PhET-iO API to be able to vary independently from the sim
implementation instead of leaking sim implementation details (like MultilineText vs Text should both just be TextIO).
Still, for a well-designed simulation, TypeIOs will often match closely with the sim types. To ensure good IO type
inheritance hierarchies follow these principles:

• factor out duplicated code or responsibilities
• have the sim developer involved in instrumentation
• make sure everything is reviewed

See phetsims/beers-law-lab#213 for more context on prior problems in this area and discussion
about it.

Also note that the since work completed in https://github.com/phetsims/phet-io/issues/1398, PhET-iO interframe
communications run on structured cloning (via PostMessage), and not just JSON strings. This means that a failure to
implement a proper toStateObject in the TypeIO will result in a hard fail when trying to send instances of that type
to the wrapper side. The error will likely look something like this: "postMessage error from phetioCommandProcessor.send"
(from phetioCommandProcessor.send()).

The Data Stream

• Create Emitter instances as appropriate to augment the data stream.
• Instrumented Emitters and Property instances naturally emit to a structured data stream and are probably what you need.
  If you need something more custom, you can call PhetioObject.phetioStartEvent and PhetioObject.phetioEndEvent directly.
• Disabled components should not deliver events, even when clicked. Change them to be pickable=false when disabled.
  See https://github.com/phetsims/phet-io/issues/282
• New code should use Emitter.addListener instead of Events.onStatic
• To suppress an Emitter.emit argument, you may specify VoidIO for its type, see PressListener.js. NOTE: this is
  not ideal, and if you need to do this, please consult with a member of the phet-io team before proceeding.
• When instrumenting new types, make sure that events are marked as phetioEventType: 'user' for pointer events, keyboard events and UI events
  (like checkbox toggled, button pressed), and phetioEventType: 'model' for model actions/responses. This is easiest to
  test in the console: colorized wrapper. Model events will be logged in black, and user events will be logged blue. You can also
  go to the data-stream wrapper to see events in JSON form. If your simulation only leverages existing model types (like
  Property/Emitter) and UI types (like sun components), then you will not be instrumenting new types.

Support dynamic state

For simulations that have static content (such as a fixed number of objects and properties), instrumentation
is mostly complete and you can skip this section. For simulations that have a dynamic number of objects,
such as Circuit Construction Kit circuits or Molecules and Light photons, the containers and elements must be instrumented.
This is currently tricky with PhET-iO. Some sims may wish to avoid this entire hassle by pre-allocating all of the
instrumented instances. Consider adding flags to indicate whether the objects are "alive" or "in the pool".

Details about how to support dynamic state.

Note: TANDEM/Group is under development in https://github.com/phetsims/phet-io/issues/1454 to make it easier to instrument
dynamic elements.

Until that is complete, Beer's Law Lab and Charges and Fields demonstrates how this may be done. A container class defines two methods:
clearChildInstances which empties a container and addChildInstance which repopulates a container one element at a
time. For example, see ShakerParticlesIO in the beers-law-lab instrumentation.

When state is set, first the container is cleared, then children are created. Child states can be obtained from toStateObject
and set back with fromStateObject, with an additional call to setValue in case additional data is supplied, or custom
code can be used.

Dispose must be implemented properly on all dynamic instances, or else it will result in stale values in the playback sim.
For example, if a simulation is sending the position of a particle as a property, if the particle position property
hasn't been disposed of, the simulation will try to create a new property with the same id and hence throw an assertion
error because that tandem is already registered.

On January 11, 2017 ControlPoints were not being disposed correctly in Energy-skate-park-basics, causing a mysterious bug
(impossible set state), make sure that children are being disposed correctly before creating them in the downstream sim!

Other tips and tricks for "impossible set state":

• addChildInstance must return the instance, it is used as a flag to determine whether addition was successful
• the given tandems must be reused. Do not use GroupTandem to assign a new tandem, use the specified tandem so the object
  can be addressed the same way

Dispose functions must be added to types that are instrumented. But that's only half of the memory management issue. The
other half is revisiting memory management for all instances that don't exist for the lifetime of the sim, and verifying
that tandems are properly cleaned up.

Two types of serialization

Data type serialization For example, numbers, strings, Vector2 instances fall into this category. These values
are instantiated by fromStateObject.

Reference type serialization For example, Nodes and Properties. For example, if a simulation has one heightProperty
that exists for the lifetime of the sim then when we save the state of the sim, we save the dynamic characteristics of
the heightProperty (rather than trying to serialize the entire list of listeners and phet-io metadata. Then the
PhET-iO library calls setValue() to update the dynamic characteristics of the heightProperty without dealing with
all of Property's many attributes. The static setValue methods on TypeIOs are automatically called by PhET-iO to
restore dynamic characteristics of reference-type serialized instances. Search for toStateObject in *IO.js files for examples.

Post Instrumentation and Checks

• Make sure unused PhetioObject instances are disposed, which unregisters the tandem.
• Make sure JOIST dt values are used instead of Date.now() or other Date functions. This is necessary for
  reproducible playback via input events. Perhaps try phet.joist.elapsedTime.
• Are random numbers using phet.joist.random, and all doing so after modules are declared (non-statically)? For
  example, the following methods (and perhaps others) should not be used: Math.random, _.shuffle, _.sample, _.random.
• Like JSON, keys for undefined values are omitted in the state--consider this when determining whether toStateObject should use null or undefined values.
• Verify that the simulation works in all of the phet-io wrappers. Launch the "index" wrapper at
  http://localhost/phet-io-wrappers/index/?sim={{simulation-name}} and test all the links. To further understand what each
  wrapper exemplifies, read the description for it in the wrapper index, and launch that wrapper with a sim already
  completely PhET-iO instrumented like Faraday's Law.
• Build with grunt --brands=phet-io and test the built version by launching build/wrappers/index and testing all the links.
• Manually look through Studio to make sure that all PhET-iO Elements work as expected and are formatted correctly.
• Perform a full test for memory leaks. The benchmark dev release can be helpful here. This will help catch faulty
  tandem disposal. PhET-iO instantiates different objects and wires up listeners that are not present in the PhET-branded
  simulation. It needs to be tested separately for memory leaks. Use ?ea&brand=phet-io&phetioStandalone&fuzz to
  run with assertions, PhET-iO brand and fuzzing.
• Run phetmarks=>aqua=>Test Sims(Fast Build) with PhET-iO checked. This will help catch any simulations using the
  component you just instrumented. Next you will need to pass tandems for those cases.

Tips, Tricks, Notes, Misc

• When testing iframes in Chrome, you sometimes must hit refresh twice in order to test your code changes. This is one
  reason that testing without iframes, using the `Data: colorized" wrapper is sometimes preferable.
• Sometimes toStateObject and fromStateObject need to manage private state, so must be declared in the type itself,
  see https://github.com/phetsims/phet-io/issues/107
• When navigating to wrappers, the easiest way to get to the whole wrapper suite is through the "wrapper index." After
  a while of testing it can be annoying to have the extra step: phetmarks --> index --> desired wrapper. Instead you
  can use phetmarks to launch any individual wrapper. Note that the wrapper index in the build version is at the top level of the build dir (build/phet-io/).

Review and Publication

• The PhET-iO instrumentation should be code reviewed, create a new issue.
• The PhET-iO instrumentation should be QA tested, create a new issue.
• Update these instructions if you find them to be incomplete, inconsistent or incorrect.
• After publishing, add your instrumented simulation to the spreadsheet here: https://docs.google.com/spreadsheets/d/1pU9izdNQkd9vr8TvLAfXe_v68yh-7potH-y712FBPr8/edit#gid=0

Maintaining

• See https://github.com/phetsims/phet-io/issues/1443#issuecomment-484306552 for an explanation of maintaining a
  PhET-iO api after it has been created.

[chrisklus]

Basic tandem passing is complete on branch phetioInstrumentation.

[chrisklus]

First dev version: https://phet-dev.colorado.edu/html/energy-forms-and-changes/1.2.0-phetioInstrumentation.1/phet-io/wrappers/studio/

Some things I've noticed to discuss for 09/12/19 design meeting:

Intro Screen

• The beakers are three separate pieces instead of one Node so that things like energy chunks and blocks can go between the front and back of the beaker. This doesn't look ideal in Studio (view), but there's no way around it that I'm aware of. Having BeakerView extend node doesn't help, because the BeakerView layers are added in the parent class, not as children to BeakerView (see "View" class suffix violates PhET naming convention #252).

  UPDATE: I'll try adding Properties to link the node Properties of all three layers, then uninstrument the separate layers.

• I didn't instrument the BurnerStandNodes or HeaterCoolerNodes because the collision logic relies on the burner stands being there after startup (for example, you would not be able to hide a BurnerStandNode and then just drop a block in its empty spot). The model is instrumented, though, so the heat-cool level can be controlled.

  UPDATE: We will investigate adding a pre-launch dialog to select which elements you want to include, then update the model to respect those query params. These query params would actually remove items when setting up the sim at startup, not just hide them.

  Possible sections of the dialog:
  global?
  screen1
  screen2

  Options for customization for this screen:
  item 1 and 2 (blocks only)
  item 3 and 4 (blocks or beakers)

  Also instrument HeaterCoolerNode and BurnerStandNode, aka BurnerNode. This likely means that these should be part of the customization dialog, but I'll double check.

• I added some code to where the thermometers and corresponding nodes are created to give them unique names like temperatureAndColorSensor2. Is this our usual pattern for fixed numbers of the same thing?

  UPDATE: Yes, but let's one-index them instead of zero. We also want to rename the code instances and tandem names to thermometerN and thermometerNodeN. Also, these need some work for setting state.

Systems Screen

• The model controls the opacity of the EnergySystemElements as they come in an out of view with an opacity Property. This duplicates the functionality of the opacity Property in the corresponding view node, so I'm thinking the opacity Properties should be uninstrumented, unless we find that causes unexpected problems.

  UPDATE: Yep, uninstrument them.

• I'm excited how nicely the three *Carousel.targetIndexProperty works! @samreid suggested turning this into an EnumerationProperty so you can select named EnergySystemElements instead of indices, which sounds very nice.

  UPDATE: Yep, convert this to use EnumerationProperty.

• The fan animation is a loop of ten images, and it looks like I programmed that as one full rotation of 2pi radians. But, since this is a side angle of the fan, the animation is actually only like < pi/4 radians, so the behavior when sliding the control for fan.bladePositionProperty feels pretty strange. I feel like I should fix this so it does a full rotation over 2pi radians, and then will hopefully just need to decrease a constant elsewhere in order for the behavior to look the same as it currently does when using the sim.

• There is currently not much customization for *SelectorPanels in the view. I think our desired behavior is to be able to remove specific buttons from the panels, not just the button group as a whole. RadioButtonGroup is a common code component, so we'll have to make modifications if we want each of the buttons instrumented individually.

  UPDATE: Yes, further instrument this common code component so that individual buttons can be removed from the button group. The combination of removing a button and setting its corresponding carousel element to not visible will be how we remove elements from the second screen. Right now, turning off an element's visibility only works until the selected element is changed, and then something is making it visible again. My plan is to remove whatever in the model is changing visibility, and only let the model control opacity.

• I didn't instrument the model Belt because it only has a visibleProperty, which duplicates the visibleProperty in BeltNode.

• I instrumented all of the systems element nodes as one unit, except SunNode, which has a Clouds panel and slider. We can get more granular if we'd like (e.g. adding children to the BeakerHeater like the beaker, thermometer, and heater).

[chrisklus]

We will investigate adding a pre-launch dialog to select which elements you want to include, then update the model to respect those query params. These query params would actually remove items when setting up the sim at startup, not just hide them.

@zepumph suggested moving this customization as an optional menu dialog located in the sim along with a button to relaunch the sim once the customizations are applied. This would allow for the customization to be confined to one place, instead of every entry point for the sim. In Studio, we would need to find a way to relaunch Studio, not just the sim itself.

This pattern would also make customizing for phet brand simpler, because the website's sim page wouldn't need to know about which query params are available for a given sim version.

We'll bring this up in design meeting and discuss further.

[samreid]

@zepumph what do you think about bypassing the normal sim construction and going straight to the query parameter UI via a query parameter, like: sim.html?configure . This would still version and deploy within the sim HTML.

[zepumph]

That sounds like it is worth investigation! In the sim feels like it is most robust, and will be easiest to maintain. I still reserve the right to be outvoted if people think that this belongs outside the sim.

[jbphet]

EFAC with phet-io was published in 1.4.0. I think a lot of things can probably be checked off in this issue now, and we should consider just closing it once it has been updated.

[jbphet]

Oops, I didn't mean to actually close it. It should probably be updated a bit before we do.