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

PhET-iO Instrumentation Process Checklist #48

Closed
veillette opened this issue Sep 20, 2022 · 3 comments
Closed

PhET-iO Instrumentation Process Checklist #48

veillette opened this issue Sep 20, 2022 · 3 comments
Assignees

Comments

@veillette
Copy link
Contributor

This issue will keep track of the effort to instrument this simulation.

@veillette
Copy link
Contributor Author

veillette commented Sep 20, 2022

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

  • 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/<SHA>/ so that the specific guide you used is preserved.
  • 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.
  • Developer to gather knowledge about the instrumentation process. These topics are crucial to understanding before
    attempting to outfit a simulation with PhET-iO:
    • A general overview of PhET-iO, please read
      the Overview section
      .
    • Make sure you understand what is contained in PhET-iO API,
      see API Management
      .
    • 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.
  • 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:
    • Review open GitHub issues. Identify issues that should be addressed during instrumentation.
    • Identify places where the sim should be brought up to PhET UX standards.
    • 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:

  • If a retrofit, the developer:
    • Performs preliminary assessment of state of the code (does it conform to newer code standards)
    • Reviews open issues in the repo
  • 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

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

Initial development

  • 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.
  • 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

  • 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
  • Run grunt update in the sim repo. This will update the local files needed to run in phet-io brand.
  • 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.)
  • 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.)
  • 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 suite wrappers is vital to
    understanding the intricacies of the instrumentation process and goals (see the Wrapper Index for entire list).
  • 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.
  • 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.

  • 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.
  • For sim-specific code, do NOT specify phetioFeatured metadata. It will typically be specified by designers using
    Studio.
    See Customizing the API in Studio
    .
  • 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 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.
  • 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.
  • Make sure that Tandem naming adheres
    to Best practices for Tandem
    .
  • 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.
  • Make sure that phetioDocumentation adheres to the
    the conventions
    .
  • If instrumenting a class,
    see Instrumenting Classes
    .
  • 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.
  • 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.
  • If necessary, instrument common-code components that are not yet instrumented.
    See Instrumenting Common Code
    .
  • 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
    (memory leaks) mentioned in https://github.com/phetsims/phet-io/issues/1179. For example, if a default option creates
    a Property.
  • 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 effect 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 paramater 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 Factor out/minimize overrides files. states-of-matter#303 (comment)
  • 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.

  1. QA will create a sim-specific bug report in the sim repo.
  2. 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.
  3. Responsible developer should look at QA pipeline project board and
    determine any other sims that could be effected by this bug.
  4. 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.
  5. 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)
  6. 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.
  7. Each sim-specific issues gets cherry-picks as needed. Individual sim-specific issues can be closed before all are
    taken care of.
  8. 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.

@veillette veillette changed the title Instrumentation of Calculus Grapher. PhET-iO Instrumentation Process Checklist Sep 20, 2022
@veillette
Copy link
Contributor Author

veillette commented Sep 20, 2022

veillette added a commit that referenced this issue Sep 21, 2022
veillette added a commit that referenced this issue Sep 21, 2022
veillette added a commit that referenced this issue Sep 21, 2022
veillette added a commit that referenced this issue Sep 21, 2022
Signed-off-by: Martin Veillette <[email protected]>
veillette added a commit that referenced this issue Sep 21, 2022
Signed-off-by: Martin Veillette <[email protected]>
veillette added a commit that referenced this issue Sep 21, 2022
Signed-off-by: Martin Veillette <[email protected]>
veillette added a commit that referenced this issue Sep 22, 2022
Signed-off-by: Martin Veillette <[email protected]>
veillette added a commit that referenced this issue Sep 22, 2022
veillette added a commit that referenced this issue Sep 22, 2022
@pixelzoom
Copy link
Contributor

This checklist is not paticularly useful, out of date, not consistently used, etc. ... as reported in https://github.com/phetsims/phet-io/issues/1897. I skimmed it and we're doing the things we should be doing. So I'm going to close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants