pixelzoom commented 2 years ago

PhET-iO Instrumentation Process Checklist

Initial Steps

Gathering requirements

[ ] Create a google doc for documenting requirements
[ ] Identify internal and any client iO requirements
[ ] Determine the developer that will be instrumenting the simulation. It is best for the simulation's responsible developer 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, even a consulting role would be helpful.

Before touching any code

[x] Create a "PhET-iO Instrumentation Process Checklist" GitHub issue in the simulation repository. Copy this checklist/guide to the issue description (top issue comment) for tracking. Link back to this checklist via blob/ so that the specific guide you used is preserved.
[x] If this is a retrofit, create a baseline dev version. 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 version in the sim's PhET-iO Github issue.

Baseline versions:

https://phet-dev.colorado.edu/html/equality-explorer/1.2.0-dev.5/phet/equality-explorer_en_phet.html
https://phet-dev.colorado.edu/html/equality-explorer-basics/1.1.0-dev.2/phet/equality-explorer-basics_en_phet.html
https://phet-dev.colorado.edu/html/equality-explorer-two-variables/1.1.0-dev.2/phet/equality-explorer-two-variables_en_phet.html
[x] Developer to gather knowledge about the instrumentation process. These topics are crucial to understanding before attempting to outfit a simulation with PhET-iO:
- [x] A general overview of PhET-iO, please read the Overview section.
- [x] Make sure you understand what is contained in PhET-iO API, see API Management.
- [x] Is there a potential for dynamic elements in the sim? Make sure to talk to your designers about whether they should be dynamic or statically allocated.
[x] Design review: PhET-iO instrumentation provides an opportunity to review the condition of the sim, and make improvements to both the UX and code base. With a designer:
- [x] Review open GitHub issues. Identify issues that should be addressed during instrumentation.
- [x] Identify places where the sim should be brought up to PhET UX standards.
- [x] Identify sim-specific UI components that should be replaced with common-code UI components. Using common-code where possible allows us to leverage common-code instrumentation, and provide a consistent UX across sims.

Initial meeting

Prior to initial meeting:

[x] If a retrofit, the developer:
- [x] Performs preliminary assessment of state of the code (does it conform to newer code standards)
- [x] Reviews open issues in the repo
[x] Developer performs a "best guess" initial instrumentation to populate Studio with something. This involves at least passing tandems to many model Properties and Tandem.REQUIRED elements. See section below on development and instrumentation, but note that not all steps should be accomplished at this stage.

Brief initial meeting (developer and designer):

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. Sometimes it is preferred to have a skeleton, or developer's "best guess" before this meeting so that there is more to play with in Studio. Use your judgement!

[ ] Identify the broad goals
[ ] Identify which requirements/goals will be hard and most important (ie set initial bunny population)
[ ] Are some of the requirements desirable for PhET brand (eg via query parameters)
[ ] Create a preliminary schedule (milestones) with google calendar reminders
[ ] Evaluate any client requirements, and work these into the design document.

Development

If a retrofit

[x] Developer should perform a thorough code review.
- [x] Bring the sim up to PhET code standards, including conversion to ES6 (classes, arrow functions, etc.)
- [x] Prefer common code UI to custom implementations
- [x] Deprecated code should not be used; use latest common code components instead. Most likely these have better PhET-iO instrumentation.
- [x] If there is a branch with significant changes, consider merging before instrumentation.
- [x] Complete any planned refactorings.
- [x] Address TODOs in the code.
- [x] Identify opportunities to use/improve PhET design patterns. Consulting phet design patterns may be helpful.
- [x] If the sim uses vibe for sound, consider porting to tambo.
- [x] If significant changes were required as the result of code review, publish a new benchmark dev version.
- [x] Instrumentation and code review commits should be separate

Initial development

[x] ~During this step, please consult with the PhET-iO Instrumentation Guide to do the sim instrumentation. Create a separate github issue called "PhET-iO Instrumentation Implementation" and copy the appropriate checklist from that guide into it.~
[x] ~~New Sim -- build out according to initial requirements.~~
[ ] Tricky items postponed for discussion
[ ] Developer generates design questions to bring back to meetings
- [ ] Is there uninstrumented common code that needs design time
- [ ] Is the Studio tree structure acceptable

Getting started

[x] Add 'phet-io' to the supportedBrands field in the sim's package.json. Then run ~/github/perennial$ grunt generate-data to 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. More documentation is available in PERENNIAL/generateData.js
[x] Run grunt update in the sim repo. This will update the local files needed to run in phet-io brand.
[x] Import Tandem to main.js, see faradays-law-main.js for an example. (If the sim was created using a recent version of simula-rasa, this was handled for you.)
[x] Pass tandem instances to each screen using tandem.createTandem(...). (If the sim was created using a recent version of simula-rasa, this was handled for you.)
[x] Studio 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 suit wrappers is vital to understanding the intricacies of the instrumentation process and goals (see the Wrapper Index for entire list).
[x] You can specify the ?phetioValidation=false query parameter or specify the package.json flag phet.phet-io.validation: false to opt out of errors during initial instrumentation. Once the simulation has attained a basic level of instrumentation, validation can be turned on to discover remaining issues. Remember to run grunt update after changing package.json.
[x] If appropriate, you can remove your sim from state wrapper fuzz testing on CT by adding it manually to perennial/data/phet-io-state-unsupported.

Instrumenting Objects by passing them Tandems

This step will take you through all objects in the simulation that should be instrumented, as well as some tips and tricks for finding them and testing as you go.

Consult the PhET-iO design issue to see what features the sim should support. See PhetioObject.js for the supported PhetioObject options. Not every Tandem created needs to be passed to a PhetioObject; sometimes Tandems can be created to support organization. For example, in some sims, a collection of Properties associated with the visibility of Nodes in the view may all be instrumented under a phetioID like {{sim}}.{{screen}}.view.viewProperties. This would be preferable than having all of these Properties (which have similar functionality) directly on the view phetioID. Here viewProperties is not a PhET-iO element, but is a phetioID that nests PhET-iO elements under it.

[x] 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 to design an API that balances the power of a broad feature set while still being maintainable.
[x] For sim-specific code, do NOT specify phetioFeatured metadata. It will typically be specified by designers using Studio. See Customizing the API in Studio.
[x] Instrument user-interface components such as Checkbox, HSlider, etc.
[x] 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 subclasses have options that are specific to PhET-iO (for example units in NumberProperty) and should be added where appropriate.
[ ] Instrument all of the features identified in the simulation's "PhET-iO Instrumentation" GitHub issue.
[x] Subclass PhetioObject when you need to add features not already covered by existing types. Be careful not to shadow pre-existing attributes in PhetioObject such as tandem, isDisposed, and linkedElements.
[x] Make sure that Tandem naming adheres to Best practices for Tandem.
[x] 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 remaining.
[ ] specify the package.json "phet-io" flag "validation": true so it will be tested with validation on your working copy and on CT (or omit, as this is the default).

Feature Support

An instrumented object is not just a Tandem passed to a PhetioObject. There is other structure that needs to be added for full PhET-iO support.

[ ] Each PhetioObject should be provided an IO Type with the phetioType option. The IO Types Section can serve as a guide.
[x] Make sure that phetioDocumentation adheres to the the conventions.
[x] If instrumenting a class, see Instrumenting Classes.
[x] 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 IO Type and implementing get/set within that IO Type.
[x] Each IOType has a validator static attribute 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 IO Type'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.
[x] If necessary, instrument common-code components that are not yet instrumented. See Instrumenting Common Code
[x] Port vibe audio (if any) to tambo. See https://github.com/phetsims/vibe/issues/33 and note that PhET-iO query parameters support tambo but not vibe.
[x] Avoid instrumenting values in default options, or at least be very careful about how it is done, see the concerns (memory leaks) mentioned in https://github.com/phetsims/phet-io/issues/1179. For example, if a default option creates a Property.
[x] For UI components, consider whether to link to the underlying Property via PhetioObject.addLinkedElement.
[ ] Add support for data stream, see Data Stream.
[ ] Add support for dynamic elements, see Dynamically Created PhET-iO Elements.
[ ] Add support for saving and loading a simulation with PhET-iO states, see Save and Load.

Iteration during development

[ ] ~With working studio carefully designers carefully review tandem structure and studio tree~ (?!?!?!)
[ ] In depth design meetings (developer not always needed):
- [ ] Generate github issue(s) with change requests
- [ ] Iterative back and forth with developer, github and meeting discussions as needed.

Post Instrumentation Review and Checks

[ ] A nice way to check state is to look at the "changed state" feature in the state wrapper. This displays only the difference between the state at startup and the current state of the sim. Before continuing, make sure that the changed state makes sense. If it looks like initial values are leaking into the changed state, then it is possible that initialization has not completed by the time the sim says that construction has ended. In most cases this is a code smell, and could also be a sneaky bug because we want to make sure that by the time the wrapper gets the onSimInitialized callback, that the sim has actually been initialized. If there's any animation on startup which causes changed state, that is expected and okay. See https://github.com/phetsims/phet-io/issues/1555 for more discussion.
[ ] 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 the compiled Wrapper Index at build/phet-io/, 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 version can be helpful here. This will help catch faulty Tandem disposal and other problems. brand=phet-io instantiates different objects and wires up listeners that are not present in the brand=phet runtime. 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.
[ ] Performance testing should also ensure usability, and, if retrofit, that no large regressions have taken place.
[ ] If you made adjustments to common code that could affect other sims, run phetmarks=>aqua=>Fuzz Test PhET-iO Sims (Fast Build). This will help catch any simulations using the component you just instrumented. Next you will need to instrument those cases.
[ ] If you turned off validation via ?phetioValidation=false or specify the package.json flag phet.phet-io.validation: false, time to turn validation back on (by removing the query parameter or package.json entry) and address any issues discovered.
[ ] Review the "overrides" file, and potentially move phetioDocumentation over to the sim from the file. All phetioFeatured metadata should be in overrides file, and not in sim-specific code. phetioFeatured can be declared in common code to factor out duplication. See overrides convention decision in https://github.com/phetsims/states-of-matter/issues/303#issuecomment-653139520
[ ] The PhET-iO instrumentation should be code reviewed. Please create a new issue for this.
[ ] The PhET-iO instrumentation process (this file!) should be code reviewed, feel free to change as desired.
[ ] Once a sim is fully designed, add phetioDesigned: true to ensure that any changes to the API don't sneak in.

Publication

[ ] Conduct a dev test with QA. The PhET-iO publication process is often quite different because it can be client-driven.
[ ] After initial dev test, further publications may be necessary depending on the specific use. Talk to the designer or project lead for more information.
[ ] If delivering a dev version to the client see Initial dev release to client (Note: you may be able to combine the initial dev test with one needed for this step).
[ ] If moving on to RC, create a QA RC test template issue and include the PhET-iO section.
[ ] Please make changes or create an issue if you find these instructions to be incomplete, inconsistent, or incorrect.
[ ] After publishing, add your instrumented simulation to the spreadsheet here: https://docs.google.com/spreadsheets/d/18_QNGuVtYtxOEKG9xRBs_PSQpyvzySF1Gk5puR-5Fv4/edit#gid=1881767354
[ ] After publishing, make sure that the active branches in the "PhET-iO Deploy Status" panel in the admin page on phet.colorado.edu is appropriate. Old versions should be set to inactive if they are no longer needing maintenance. The fewer the active release branches per sim, the lower the maintenance cost, but make sure that old versions aren't being used by "important clients."

Managing QA Bugs with PhET-iO publication

PhET-iO bugs that come from QA testing can effect multiple sims, especially if multiple phet-io sims are currently going through the QA pipeline. Below is a process to follow to make sure that bugs can be fixed and propagated to all effected sims.

QA will create a sim-specific bug report in the sim repo.
Responsible developer (whoever is bringing that sim through QA) should determine if this is a common code issue, and, if so, create another issue in that common code repo. This issue should have the exact same name as the sim-specific one.
Responsible developer should look at QA pipeline project board and determine any other sims that could be effected by this bug.
Assign each responsible dev to determine if it applies to their sim, and if so to create their own sim-specific issue for it. This should be done regardless of if this bug blocks the publication of that sim.
Each sim-specific issue gets “is blocking” triage in their own context (from their designer) (likely marking on hold until the general issue is solved)
Common code issue gets fixed, with help from design team if needed. This issue can get closed even if the change hasn't been propagated to all sims/versions.
Each sim-specific issues gets cherry-picks as needed. Individual sim-specific issues can be closed before all are taken care of.
When is the next RC? When there are no blocking-sim-publication issues in that repo. Also make sure that there are no general blocks-publication issues that aren’t covered by your sim-specific issues.

pixelzoom commented 2 years ago

Note that the "Before touching any code" section says:

Design review: PhET-iO instrumentation provides an opportunity to review the condition of the sim, and make improvements to both the UX and code base. With a designer: ...

I'm skipping that for this sim, because I've been asked to proceed, and there are no outstanding GitHub issues that I think are relevant here. @amanda-phet if you feel otherwise, please let me know asap.

pixelzoom commented 2 years ago

Good progress on "initial instrumentation" today in the above commits. I'll resume working on this the week of 10/17/2022.

pixelzoom commented 2 years ago

Dynamic elements have been identified in https://github.com/phetsims/equality-explorer/issues/200. There are no plans to work on that until the initial instrumentation has been reviewed.

pixelzoom commented 2 years ago

I've completed initial instrumentation.

Baseline versions:

Nexts steps are for designers @amanda-phet @catherinecarter - see https://github.com/phetsims/equality-explorer/issues/191#issuecomment-1306323892.

phetsims / equality-explorer