Migration rules for 1.5 release

[pixelzoom]

319 (add support for dynamic locale) is requiring a large number of API changes. According to the PhET-iO spreadsheet, this sim needs to provide migration support from 1.4 to 1.5. So migration rules will be needed.

[pixelzoom]

We need to discuss when this happens, who is doing the work, etc. Should it be before/done after PhET-iO design review, or iteratively? Am I doing this with support from the PhET-iO team, or is the PhET-iO team writing migration rules?

[pixelzoom]

319 (add support for dynamic locale) is completed. Design review #323 has not been done yet.

Ready to discuss with @samreid and @zepumph.

[zepumph]

@samreid and I ran out of time for this today, but will look into it soon. We will likely need to add a couple more migration rules due to recent changes (removing the options dialog, etc).

[pixelzoom]

At 9/1/2022 design meeting, @kathy-phet said:

• migration rules should happen after #323
• @samreid and @zepumph will be doing an initial pass at migration rules
• @samreid and @zepumph might want to do a quick "scope check" by loading a previous-version state file into master

[samreid]

I tried this with a copy/paste of the Gravity and Orbits rules but was blocked by a "schema expected something that it didn't receive".

[samreid]

I added a rough draft of rules in the commit. I attempted to factor out the shared rules between it and gravity and orbits, but ran into 2 problems:

• Should those be in phet-io-wrapper-sim-specific or phet-io-wrappers? I wanted to add it to the latter since that is where the rules are defined, and there is no "repos/" folder for these. However, having the rules cross repos would make it harder to move rules back and forth between sims and the common list.
• The rules are order-dependent so my initial idea to do naturalSelectionRules =[...commonRules, ...]` won't work because as common rules accumulate they will always be before the sim-specific rules. Instead, we may need to add a timestamp, index or other ordering metadata so common code rules can be interspersed with sim-specific rules. But this sounds very brittle and will make it difficult to read the rule list.

Until we resolve these considerations, each sim can just have a copy-paste list of the common rules.

Next I'll try to relax some of the deletes.

[samreid]

I wrote up notes on the rules that are deleted. It's at a good point to discuss with @zepumph, possibly on Tuesday.

[kathy-phet]

I don't think its a bad idea to get a handle on what is involved here, but as Chris mentioned, please do not put too much effort into Natural Selection specific rules until after the design review is complete. @arouinfar and I need to take a good look at the changes that happened since the last published version and where the simulation is now. We may decide its worth uninstrumenting some things, despite the cost of that.

[zepumph]

Discussed with @samreid. On hold until we complete the natural selection design overhaul in https://github.com/phetsims/natural-selection/issues/323.

[zepumph]

Unassigning until we work on this sim

[pixelzoom]

Migration rules for Molecule Polarity https://github.com/phetsims/molecule-polarity/issues/141 was quite the project. @samreid suggested that, while it's still fresh in my mind, I should do this sim. @kathy-phet approved. So I'll proceed.

[pixelzoom]

Before starting this work, I tested in master with the Migration wrapper with URL: http://localhost:8080/phet-io-wrappers/version-migration/?sim=natural-selection&locales=*&keyboardLocaleSwitcher&phetioDebug=true&phetioWrapperDebug=true&oldVersion=1.4

There is 1 error reported in the console:

Model strings are not stateful, instead use ConvertToStringsStateForLocale: naturalSelection.general.model.strings.naturalSelection.naturalSelection.titleStringProperty

I also published a baseline version:

• https://phet-dev.colorado.edu/html/natural-selection/1.5.0-dev.3/phet/natural-selection_all_phet.html
• https://phet-dev.colorado.edu/html/natural-selection/1.5.0-dev.3/phet-io/

[pixelzoom]

In the above commits, I added all rules needed to stop the Migration wrapper from complaining to the console.

Now I'm getting this console output after pressing the "Migrate Now" button:

Building a MigrationEngine with 126 rules.
Assertion failed:  stateObject is not valid for range
PhetioStateEngine.ts:342 Uncaught Error: Assertion failed: stateObject is not valid for range
    at window.assertions.assertFunction (assert.js:28:13)
    at StateSchema.ts:147:35
    at Array.forEach (<anonymous>)
    at StateSchema.checkStateObjectValid (StateSchema.ts:140:29)
    at IOType.isStateObjectValid (IOType.ts:403:43)
    at IOType.validateStateObject (IOType.ts:436:10)
    at IOType.applyState (IOType.ts:298:41)
    at PhetioStateEngine.setStateForPhetioObject (PhetioStateEngine.ts:424:10)
    at PhetioStateEngine.ts:327:14
    at Array.forEach (<anonymous>)

I'm running the Migration wrapper from phetmarks with oldVersion=1.4, and this is the URL:

http://localhost:8080/phet-io-wrappers/version-migration/?sim=natural-selection&locales=*&keyboardLocaleSwitcher&phetioDebug=true&phetioWrapperDebug=true&oldVersion=1.4

@zepumph @samreid any idea what's going on here, and how to fix it?

[pixelzoom]

The above commit fixes the 'stateObject is not valid for range', which was related to NumberProperty's rangeProperty. It also fixes the problem that NumberProperty step no longer exists.

Now that we've cleared those hurdles, here's the next problem:

Assertion failed: _restTime in state schema but not in the state object

This is due to a change in how "private" fields (hidden from Studio) are now implemented in an IOType's state schema. In this case, the IOType is BunnyIO. WolfIO will also have this problem. @samreid said that we'll need to implement migration rule to "rewrite" the schema. Before doing that, we might want to address #327 and #330. Or we might want to write these rules now, to discover whether there are other problems that haven't been reported yet.

[samreid]

@samreid said that we'll need to implement migration rule to "rewrite" the schema.

Another way to phrase it would be: a migration rule to promote values that match the old schema to values that match the new schema.

[pixelzoom]

Completion is also blocked by:

• https://github.com/phetsims/phet-io-wrappers/issues/513

[zepumph]

https://github.com/phetsims/phet-io-wrappers/issues/514 is no longer blocking this issue.

[zepumph]

Completion is also blocked by:

Further work on this issue is no longer blocked by https://github.com/phetsims/phet-io-wrappers/issues/513, but that issue is still out for review.

Here you can still continue review and testing. Likely you will need the same AboutDialog rules from MoleculePolarity.

[pixelzoom]

In the above commit, I added rules related to aboutDialog, copied from molecule-polarity-migration-rules.ts.

[pixelzoom]

The next steps for this will be to address IOType issues (#327 and #330), then revise migration rules.

Unassigning until this sim is added to the worklist.

[pixelzoom]

For https://github.com/phetsims/qa/issues/967 please test with the Migration wrapper. Then assign back to me.

The previous version is https://phet-io.colorado.edu/sims/natural-selection/1.4/

[Nancy-Salpepi]

The Migration wrapper was tested on multiple platforms in https://github.com/phetsims/qa/issues/967. Back to you.

[pixelzoom]

For https://github.com/phetsims/qa/issues/968, please test with the Migration wrapper. Then assign back to me.

[KatieWoe]

Things look ok in rc.1

[pixelzoom]

For https://github.com/phetsims/qa/issues/971, please test with the Migration wrapper. Then assign back to me.

[KatieWoe]

Migration wrapper looked ok in rc.2

[pixelzoom]

We're ready for production deploy of 1.5.0, so it's now safe to close this issue.