Instrument for PhET-iO

[zepumph]

This was mentioned as a possible sim for a11y work and sonification. So I'll create an issue to track any instrumentation. It already has a pretty good harness (root tandem passed to model/view).

[zepumph]

Initial instrumentation is done to make sure that the sim runs. I didn't do much to it.

We need to make sure we handle common code components correctly. These are the components I just removed the uninstrumented marker from:

• NumberPicker is instrumented as we like it ( I passed tandem to many properties and the state looks good)
• FireOnHoldListener needs to be instrumented.

[samreid]

@kathy-phet said:

After discussion with Emily today, we would like to move forward with the instrumentation of Gravity Force Lab: Basics as a high priority, and are hoping that you could turn your attention to that over the next few work days. [...]
We would instrument under the older paradigm of basically instrumenting everything (but use your judgement if you think something just won't be needed).
[ ...]
Please consult with Jesse as needed, if you have questions about the sim itself.
Let me know if this seems OK or you have any concerns about instrumentation. Also, if you could also let me know how long it takes in the end, that would be great.

[samreid]

I can see that @zepumph already worked on this. There are 136 instances in instance proxies. I'll look through status, cleanup what's here and see what other work is necessary, if any.

[samreid]

@kathy-phet and @emily-phet do you want to do a full QA test cycle on this? Do you want it to be published on the PhET-iO website? Do you need a full code review before it can be used?

[kathy-phet]

Sam, Priorities right now would be to bring the PhET-iO pieces up to current code approaches (except for not doing the design step), and then having Steele do some sanity testing on the phet-io pieces.

We won't want to publish it for PhET-iO clients until Jesse has finished and the entire sim has been code reviewed and tested through normal rc channels. But when that does come to pass and we are publishing it to the main website, I would like to publish a phet-io version of the sim (even if we haven't done the design review approach that is our new optimal approach). Adding a sim for clients will be useful.

[samreid]

Understood, thanks! It looks like one of the main steps in moving forward will be to extend PhetioObject instead of tandem.addInstance directly.

[emily-phet]

@jbphet Since you're aware of both the PhET-iO side of things, sonification, and how Mike/Ashton have been doing their design work, can you comment if any of these derived quantities might need something more direct created?
I'm just imagining that if it's simple to make those derived quantities readily available in PhET-iO, it may save a lot of downstream work for sonification and/or other users of PhET-iO. I don't know the details, so it could be trivial and not necessary to do in PhET-iO.

[samreid]

@mikewinters10 do you need to distinguish between (a) when the user drags one mass into the other mass vs (b) when the masses collide and push back due to increased radius vs (c) when the user drags one mass to the edge of the screen?

[samreid]

@jessegreenberg and I determined that there is already an instrumented property enabledRangeProperty used for accessibility which would likely provide the needed information to know when the masses reach the end of their range. We will try to move it from the view to the model.

[emily-phet]

@samreid When user drags puller/mass and bumps into edge of screen the user will feel like they've bumped the "puller" into edge of screen, not the mass. This should definitely be distinct in phet-io from masses bumping into each other.

I don't think it's necessary to distinguish between masses bumping into each other by dragging versus change in size. Conceptually the outcome is the same, so I think we'd always want to cue that this occurred the same way.

[emily-phet]

@samreid Wait, I'm changing my mind on one thing. I think a, b, and c, are all unique cases. C is unique as described in my last comment. A and B are distinct because in B you can actually push back the opposite mass but you cannot do that in A, so there is something conceptually distinct about these two scenarios that we might need to cue in different ways with sound.

[samreid]

@jessegreenberg and I moved the enabledRange to the model and it seems to be working properly. We also discussed tradeoffs between coding logic in the sim vs wrapper for detecting collision with the other mass vs wall and based on movement vs radius changing, and we agreed it seems simple enough (and superior) to distinguish between these on the wrapper side. Here is wrapper pseudocode:

if (mass2.enabledRange or mass2.position changed){ 
	if (mass2.value===mass2.enabledRange.max) // wall hit detected
	if (mass2.value ===mass2.enabledRange.min) // ball hit detected
}

if (mass1 radius changed or mass2 radius changed){
	if (mass1.value===mass1.enabledRange.min) // increased radius caused collision
	if (mass1.value ===mass1.enabledRange.max) // increased radius caused collision
}

[samreid]

@jessegreenberg and I determined the simulation is sufficiently PhET-iO instrumented for a first dev test. @jessegreenberg said there are a few issues in inverse-square-law-common to tackle before first dev test, but that he can start working on those and create a dev version when ready. A reminder that the dev test should include phet-io testing. Over to you @jessegreenberg

[emily-phet]

So fast! Thanks @samreid

[jessegreenberg]

This sim is ready for a dev test. I ran into an issue with instance-proxies with the phet-io build, documented in the above issue. Once that is resolved we can start testing the phet-io version.

[jbphet]

@emily-phet said, "Since you're aware of both the PhET-iO side of things, sonification, and how Mike/Ashton have been doing their design work, can you comment if any of these derived quantities might need something more direct created?"

The main I would be concerned about would be the velocity of the ball, since deriving the value will require time-based repeated actions in the wrapper. However, I'm also not convinced that we would want sonic information for both the velocity and the position, and velocity isn't very meaningful in this sim. I'd suggest we start with what @samreid and @jessegreenberg have added and see how it works.

The other issue is hitting boundaries. @samreid requested help from @jessegreenberg on this one, but I didn't see any comments that directly address it.

[samreid]

Good point about the velocity @jbphet, I'd also be concerned about how that would be represented in a wrapper. @jessegreenberg and I commented on the boundaries in #22 (comment)

[mikewinters10]

As for velocity, @samreid & @jbphet -- I'm planning on using velocity to convey the sound of the balls dragging on the ground. The sound would change in volume depending on the velocity of the ball, like the sound of everyday kinetic friction. In BASE, we derived velocity from position, and we could do it again here. As a user, my preference would be that it would just be available without derivation. It seems like the trade off would be computational load / complexity on the PhET-iO backend vs. every user that needs the quantity having to derive it themselves. And @samreid, I agree with having (a), (b), (c) as distinct cases. Best, Mike

…

On Tue, Jan 9, 2018 at 12:43 PM, Sam Reid ***@***.***> wrote: Good point about the velocity @jbphet <https://github.com/jbphet>, I'd also be concerned about how that would be represented in a wrapper. @jessegreenberg <https://github.com/jessegreenberg> and I commented on the boundaries in #22 (comment) <#22 (comment)> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#22 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AOI6zhM9FFwbswxu21HpJsQlVR8Rr6nqks5tI6UrgaJpZM4QzGKW> .

[samreid]

While reworking VerticalCheckBoxGroup for Capacitor Lab Basics phetsims/capacitor-lab-basics#220 I noticed the checkboxes are not instrumented. I don't think it's essential for the first dev version, but I'll let you know when I've added instrumentation for the checkboxes.

[jessegreenberg]

Thanks @samreid. Initial dev test created here: phetsims/qa#83

[emily-phet]

@jessegreenberg can you comment on the status of dev testing? It looks like a first round was complete...what are the next steps?

[jessegreenberg]

It looks like all issues that came out of the dev test have been fixed and are pending testing. @mbarlow12 is going to prepare a new dev test for this sim and the QA team will verify that issues have been fixed. Then this repo and inverse-square-law-common will get a code review. Once review issues have been resolved this sim will be ready for release candidate testing prior to deployment.

Since @mbarlow12 is going to prepare the next test, assigning to him.

[mbarlow12]

New dev test issue created: phetsims/qa#88.

[mbarlow12]

@samreid @phet-steele is there a reason I shouldn't close this issue? If not, I think we can create a new dev release. Thanks.