Ignore irrelevant harmonics?

[pixelzoom]

This came up during discussion of phetsims/mean-share-and-balance#60 (use of static vs dynamic PhET-iO elements).

Fourier has not been PhET-iO designed yet, and the PhET-iO implementation is incomplete. It uses a static set of 11 harmonics. When numberOfHarmonicsProperty is set to N, all harmonics > N have their amplitude set to 0, ensuring that they have no contribution to the series' waveform. Our decisions for using a "static elements" approach are captured in #6.

In a Zoom discussion with @samreid and @marlitas, @samreid pointed out that (via Studio) you can set amplitudes for harmonics > N, and they will incorrectly contribute to the waveform. I will investigate how this relates to the desired PhET-iO design for this sim, and whether anything needs to be done here.

@arouinfar FYI.

[pixelzoom]

@arouinfar In #123 (comment), you said:

@pixelzoom I would consider the design review to be complete. Change requests are summarized in #123 (comment) with game-specific requests in #144.

So are you OK with the PhET-iO client being able to change harmonics that do not have a visible slider? For example:

1. Run the sim in Studio
2. Go to Discrete screen
3. Change Harmonics spinner to 5
4. In Studio, set fourierMakingWaves.discreteScreen.model.fourierSeries.harmonics.harmonic11.amplitudeProperty to 1. Harmonic 11 is now non-zero, so it's shown on the Harmonics chart, and contributes to the Sum. See screenshot below.

I vaguely recall discussing this long ago. Maybe we decided that it's OK as is, and that we'd not in the Client Guide "Do not set non-zero amplitudes for harmonics that are not visible." That would certainly be easier than preventing client from doing that (which does not seem doable, because phetioReadOnly is not dynamic) or changing the implementation to ignore those harmonics.

[pixelzoom]

@arouinfar I see in #123 (comment):

Remove Studio Control

• discreteScreen.model.fourierSeries.harmonics.harmonic*.amplitudeProperty
• waveGameScreen.model.level*.answerSeries.harmonics.harmonic*.amplitudeProperty
• waveGameScreen.model.level*.numberOfAmplitudeControlsProperty

So we're not planning to allow amplitudes to be changed via Studio. If that's the case, then perhaps they should also be phetioReadOnly: true?

[pixelzoom]

If we want to prevent the PhET-iO client from sabatoging themselves by setting a non-zero amplitude for irrelevant harmonics... We could consider adding visibleProperty to Harmonic, which is derived from numberOfHarmonicsProperty. Harmonics that are invisible would effectively have zero amplitude, regardless of the value of their amplitudeProperty. For view components related to a Harmonic (amplitude sliders, measurement tools, waveforms, ...) we would wire up their visibleProperty to the Harmonic's visibleProperty.

[pixelzoom]

Since a PhET-iO version of this sim is currently not on the horizon, I'm going to label this issue as deferred, and unassign myself and @arouinfar. We'll come back to this when PhET-iO work resumes.

[arouinfar]

@arouinfar I see in #123 (comment):

Remove Studio Control

• discreteScreen.model.fourierSeries.harmonics.harmonic*.amplitudeProperty
• waveGameScreen.model.level*.answerSeries.harmonics.harmonic*.amplitudeProperty
• waveGameScreen.model.level*.numberOfAmplitudeControlsProperty

So we're not planning to allow amplitudes to be changed via Studio. If that's the case, then perhaps they should also be phetioReadOnly: true?

These elements should not be phetioReadOnly: true. The reason for asking for phetioStudioControl: false was due to the slider being problematic. Now that we have a better data entry interface in Studio, this set of requests is no longer necessary.