Add phetioDesigned:true

[pixelzoom]

For phetsims/qa#662

In phetsims/gravity-and-orbits#386 (comment), I asked:

If version matches package.json for GAO, why is that not the case for Natural Selection?

@samreid replied:

Gravity and Orbits specifies phetioDesigned: true for the entire simulation (in main), but Natural Selection does not. Perhaps this would be a good time to enable phetioDesigned tracking for natural selection?

[pixelzoom]

@samreid can you refresh my memory on what phetioDesigned is, and how to set it?

@amanda-phet is this something that we want to do for Natural Selection?

And do we want to do this in master, the 1.4 branch, both? @samreid what do you recommend?

[samreid]

I updated the documentation in https://github.com/phetsims/phet-io/blob/master/doc/phet-io-instrumentation-technical-guide.md#post-publication-steps to clarify some of the questions you raised. Please review that, and refer to gravity-and-orbits-main.js as an example if necessary. Please let me know if there are more questions or problems.

[pixelzoom]

From #294 (comment):

And do we want to do this in master, the 1.4 branch, both? @samreid what do you recommend?

[samreid]

I recommend changing this in master.

[pixelzoom]

@samreid can you refresh my memory on what phetioDesigned is, and how to set it?

@samreid Thanks for the updated "post-publication" documentation, that will help as a reminder to set it up. But @amanda-phet and I are still unclear on what phetioDesigned is. Can you please summarize?

[pixelzoom]

For Natural Selection, I added phetioDesigned:true to Sim options, verified that package.json contains "compareDesignedAPIChanges": true, and ran grunt generate-phet-io-api.

In natural-selection/package.json:

  "version": "1.5.0-dev.0",

In phet-io/api/natural-selection.json:

  "version": {
    "major": 1,
    "minor": 0
  }

I see nothing about having to manually update version in https://github.com/phetsims/phet-io/blob/master/doc/phet-io-instrumentation-technical-guide.md#post-publication-steps.
But I seem to recall that this version mismatch caused a problem (in GAO?) so I have not commited and pushed my changes. @sam if I push with this mismatch, is this going to be a problem?

[pixelzoom]

@samreid I believe that the order is wrong for the steps that you added to https://github.com/phetsims/phet-io/blob/master/doc/phet-io-instrumentation-technical-guide.md#post-publication-steps. phetioDesigned: true needs to be done BEFORE generating the API json file. Please confirm that steps look correct after the above commits.

[pixelzoom]

Looking at the API files in phet-io/api/, none of them appear to have "version" that matches package.json. There was discussion about this in phetsims/gravity-and-orbits#386 (comment), but I don't understand what's going on here.

[pixelzoom]

I went ahead and pushed my changes for the 2 commits above. I have no idea how to test this, or whether the version mismatch will be a problem, so I'll keep an eye on CT.

@samreid please review, and:

• Verify that I did this correctly.
• Answer the above questions about "version" in API file vs package.json.
• Describe phetioDesigned for designers, so that they know what it does, and when/where it's appropriate to add it.

[pixelzoom]

My understanding is that this does not impact the 1.4 release, because it's only related to CT testing of master. If that's not correct, then someone please add a "block-sim-publication" label asap.

[samreid]

Looking at the API files in phet-io/api/, none of them appear to have "version" that matches package.json. There was discussion about this in phetsims/gravity-and-orbits#386 (comment), but I don't understand what's going on here.

The data type of simVersion is now NullableIO(StringIO) It is output as null in the API file, and string when running a live sim. We nulled it out because we didn't want the version number to be part of the API and require API file regeneration when changed.

Verify that I did this correctly.

Looks good, thanks!

Describe phetioDesigned for designers, so that they know what it does, and when/where it's appropriate to add it.

Marking a subtree as phetioDesigned: true means its design has been fully designed and implemented, and it triggers errors on CT if the implementation deviates.

My understanding is that this does not impact the 1.4 release, because it's only related to CT testing of master. If that's not correct, then someone please add a "block-sim-publication" label asap.

This does not block publication.

[pixelzoom]

The data type of simVersion is now NullableIO(StringIO) ....

What is simVersion? We're talking about version, which I understood from phetsims/gravity-and-orbits#386 to be the same as what's in package.json.

[samreid]

I thought this was clarified in phetsims/gravity-and-orbits#386 (comment). To recap, using master and natural selection as an example:

// this is the version of the data stream, in case we change data stream semantics/structure
"dataStreamVersion": "1.0.0", 

// placeholder for the simulation version.  Not checked in with the API files.  But is exposed during
// sim runtime so the client can get the version that way.
"simVersion": null, 

// Top level version of the API file structure
"version": { 
    "major": 1,
    "minor": 0
  }

But I seem to recall that this version mismatch caused a problem (in GAO?) so I have not commited and pushed my changes.

We resolved the mismatch by nulling out the simVersion. There was never a mismatch problem with the 1.0 "version".

[pixelzoom]

@samreid said:

Marking a subtree as phetioDesigned: true means its design has been fully designed and implemented, and it triggers errors on CT if the implementation deviates.

... and after publication, the general recommendation is to put phetioDesigned: true on the top of the Studio tree, so that the complete tree will be checked for deviations.

@amanda-phet if this answers your question, feel free to close. If you need more explanation, please ask @samreid directly.

[pixelzoom]

#294 (comment):

... Please confirm that steps look correct after the above commits.

I don't see any indication that @samreid reviewed this, so assigning to him.

[samreid]

I reviewed the changes to the phet-io-instrumentation-guide and they look good. Anything else for this issue?

[pixelzoom]

@samreid asked:

... Anything else for this issue?

That's all of the questions I had for you, thanks. @amanda-phet will contact you if she needs more clarification about phetioDesigned.

[pixelzoom]

Oh, I see that @amanda-phet gave a 👍🏻 .

Closing, since there's nothing to be done or tested for the 1.4 release branch.

[pixelzoom]

Reopening.

I've followed all of the steps in https://github.com/phetsims/phet-io/blob/master/doc/phet-io-instrumentation-technical-guide.md#post-publication-steps for setting up phetioDesigned: true. Everything is commited and pushed. When I run NS in Studio with phetioCompareAPI, it's failing with:

Assertion failed: Designed API changes detected, please roll them back or revise the reference API:
.... lots of json ....
at window.assertions.assertFunction (assert.js:25)
at XMLHttpRequest. (phetioEngine.js:312)

The json output part of the console output is way too long to be examined manually.

I've tried re-running grunt generate-phet-io-api, and no changes are generated, so there's nothing for me to diff.

@samreid @zepumph suggestions on how to proceed?

[pixelzoom]

Moving the phetioCompareAPI to a new issue, #301. Re-closing.