how to implement variable number of harmonics

[pixelzoom]

Related to #4.

The Discrete screen in Fourier allows the student to change the number of harmonics N, and set amplitudes for those harmonics. The amplitudes are labeled as A₁...A_N, and the maximum for N is 11.

There are different ways of handling a variable number of harmonics. This decision will affect many other aspects of the implementation, including the PhET-iO interface (see #4). Here are 3 approaches that are being considered.

(1) A static Waveform instance that has 11 static amplitude Properties (A₁...A₁₁)
Model and view contain elements for all 11 amplitudes. The number of amplitude Properties that are relevant is based on N. Model amplitudes > N are set to zero. View components are visible only when associated with amplitudes A₁...A_N. All PhET-iO elements are static, with model and view components for all 11 amplitudes existing for the lifetime of the sim, and therefore always visible in Studio and available to PhET-iO.

Pseudo code:

class FourierModel {
  constructor( options ) {

    options = merge( {
      tandem: Tandem.REQUIRED
    }, options );

    // @public (read-only)
    this.waveform = new Waveform( {
      tandem: options.tandem.createTandem( 'waveform' )
    } );
  }
}

class Waveform {
  constructor( options ) {

    options = merge( {
      tandem: Tandem.REQUIRED
    }, options );

    // @public
    this.numberOfHarmonicsProperty = new NumberProperty( 11, {
      numberType: 'Integer',
      range: new Range( 1, 11 ),
      tandem: options.tandem.createTandem( 'numberOfHarmonics' )
    } );

    // @public (read-only) {NumberProperty[]}
    this.amplitudeProperties = [];
    for ( let i = options.numberOfHarmonicsRange.min; i <= options.numberOfHarmonicsRange.max, i++ ) {
      this.amplitudeProperties.push( new NumberProperty( 0, {
        range: new Range( -1.27, 1.27 ),
        tandem: options.tandem.createTandem( `amplitude${i}Property` )
      } ) );
    }

    // Set irrelevant amplitudes to zero.
    this.numberOfHarmonics.link( numberOfHarmonics => {
      for ( let i = numberOfHarmonics; i < this.amplitudeProperties.length; i++ ) {
        this.amplitudeProperties[ i ].value = 0;
      }
    } );
  }
}

(2) A static Waveform instance with a dynamic group of amplitude Properties (A₁...A_N)
When N changes, the number of amplitude Properties in the Waveform instance changes. This approach requires a PhetioGroup for managing the amplitude Properties, with tandem names numbered from 1...N. (Does PhetioGroup support numbering from 1, and reusing indices?). View components associated with the amplitude Property instances will also be managed using PhetioGroup.

Pseudo code:

class FourierModel {
  constructor( options ) {

    options = merge( {
      tandem: Tandem.REQUIRED
    }, options );

    // @public (read-only)
    this.waveform = new Waveform( {
      tandem: options.tandem.createTandem( 'waveform' )
    } );
  }
}

class Waveform {
  constructor( options ) {

    options = merge( {
      tandem: Tandem.REQUIRED
    } );

    // @public
    this.numberOfHarmonicsProperty = new NumberProperty( 11, {
      numberType: 'Integer',
      range: new Range( 1, 11 ),
      tandem: options.tandem.createTandem( 'numberOfHarmonics' )
    } );

    // @public (read-only) {PhetioGroup.<NumberProperty>}
    this.amplitudePropertiesGroup = new PhetioGroup( ... );

    // Add or remove amplitude Properties
    this.numberOfHarmonicsProperty.link( numberOfHarmonics => {
      //TODO add or remove from this.amplitudePropertiesGroup
    } );
  }
}

(3) A dynamic Waveform instance with N amplitude Properties (A₁...A_N)
When N changes, a new Waveform instance is created. The N amplitude Properties will always have the same tandem names, indexed 1...N. Amplitude values from the previous Waveform are used to initialize the new Waveform instance, with amplitudes for harmonics > N being set to zero. View components associated with the Waveform instance will either need to be mutable or managed using a PhetioGroup/PhetioCapsule.

Pseudo code:

class FourierModel {
  constructor( options ) {

    options = merge( {
      tandem: Tandem.REQUIRED
    }, options );

    // @public
    this.numberOfHarmonicsProperty = new NumberProperty( 11, {
      numberType: 'Integer',
      range: new Range( 1, 11 ),
      tandem: options.tandem.createTandem( 'numberOfHarmonicsProperty' )
    } );

    // @public {DerivedProperty.<Waveform>}
    this.waveformProperty = new DerivedProperty( [  this.numberOfHarmonicsProperty ], 
     numberOfHarmonics => new Waveform( numberOfHarmonics, {
       //TODO use amplitudes from previous value to initialize new value
      tandem: options.tandem.createTandem( 'waveform' )
     } )
    }, {
      tandem: options.tandem.createTandem( 'waveformProperty' )
    } );
}

class Waveform {
  constructor( numberOfHarmonics, options ) {

    options = merge( {
      tandem: Tandem.REQUIRED
    } );

    // @public (read-only)
    this.amplitudeProperties = [];
    for ( let i = 1; i <= numberOfHarmonics, i++ ) {
      this.amplitudeProperties.push( new NumberProperty( 0, {
        range: new Range( -1.27, 1.27 ),
        tandem: options.tandem.createTandem( `amplitude${i}Property` )
      } ) );
    }
  }
}

[pixelzoom]

Labeling for discussion at design meeting, and it would be good to have someone from the PhET-iO team attend.

[pixelzoom]

Assigning to @arouinfar @ariel-phet and @kathy-phet so they can think about this before design meeting, and provide input before the meeting if they feel like it.

[pixelzoom]

@ariel-phet asked me to summarize the pros and cons of each approach. And I think it's useful to show the (hypothetical) structure of the Studio tree. So here goes...

(1) A static Waveform instance that has 11 static amplitude Properties (A₁...A₁₁)

PROS:

• Less-complex, static view components, that change visibility based on number of harmonics.
• PhET-iO instrumentation is relatively straightforward.

CONS:

• Many elements shown in the Studio tree are not relevant, depending on the value of numberOfHarmonicsProperty.
• Many opportunities for client to do things that conflict with numberOfHarmonicsProperty -- for example, if numberOfHarmonicsProperty is 3 and they set the value of amplitude6Property or make amplitude6Slider visible.

Studio Tree:

Looks like this, regardless of the value of numberOfHarmonicsProperty:

-fourier-making-waves
 -discreteScreen
  -model
   -waveform
     +numberOfHarmonicsProperty {NumberProperty}
     +amplitude1Property {NumberProperty}
     +amplitude2Property {NumberProperty}
     +amplitude3Property {NumberProperty}
     +amplitude4Property {NumberProperty}
     +amplitude5Property {NumberProperty}
     +amplitude6Property {NumberProperty}
     +amplitude7Property {NumberProperty}
     +amplitude8Property {NumberProperty}
     +amplitude9Property {NumberProperty}
     +amplitude10Property {NumberProperty}
     +amplitude11Property {NumberProperty}
  -view
    -amplitudesPanel
      -amplitudesChart
       +amplitude1Slider
       +amplitude2Slider
       +amplitude3Slider
       +amplitude4Slider
       +amplitude5Slider
       +amplitude6Slider
       +amplitude7Slider
       +amplitude8Slider
       +amplitude9Slider
       +amplitude10Slider
       +amplitude11Slider
    -harmonicsAccordionBox
      -harmonicsChart
       +harmonic1Plot
       +harmonic2Plot
       +harmonic3Plot
       +harmonic4Plot
       +harmonic5Plot
       +harmonic6Plot
       +harmonic7Plot
       +harmonic8Plot
       +harmonic9Plot
       +harmonic10Plot
       +harmonic11Plot
    -sumAccordionBox
      -sumChart
        +sumPlot

---

(2) A static Waveform instance with a dynamic group of amplitude Properties (A₁...A_N)

PROS:

• Only relevant elements appear in the Studio tree.

CONS:

• More complicated view components that must adapt to changing number of amplitude Properties.
• More complicated PhET-iO instrumentation, because it involves PhetioGroup for dynamic model and view elements.
• Many dynamics elements means that client's changes to elements can more easily be blown away, or a more complicated implementation must attempt to preserve them.
• Tandem name indexing that is corresponds to desired harmonic indices is probably not currently supported by PhetioGroup, see PhetioGroup tandem name indexing tandem#226. So there's likely some core PhET-iO work required for this approach.
• If we choose this approach, we should use PhetioGroup now, instead of having to replace new later.

Studio Tree:

Looks like this for numberOfHarmonicsProperty = 3:

-fourier-making-waves
 -discreteScreen
  -model
    -waveform {Waveform}
     +numberOfHarmonicsProperty {NumberProperty}
     -amplitudePropertyGroup {PhetioGroup}
       +amplitude1Property {NumberProperty}
       +amplitude2Property {NumberProperty}
       +amplitude3Property {NumberProperty}
  -view
    -amplitudesPanel
      -amplitudeChart
        -amplitudeSliderGroup {PhetioGroup}
         +amplitude1Slider
         +amplitude2Slider
         +amplitude3Slider
    -harmonicsAccordionBox
      -harmonicsChart
       -harmonicPlotGroup {PhetioGroup}
         +harmonic1Plot
         +harmonic2Plot
         +harmonic3Plot
    -sumAccordionBox
      -sumChart
        +sumPlot

---

(3) A dynamic Waveform instance with N amplitude Properties (A₁...A_N)

PROS:

• Only relevant elements appear in the Studio tree.

CONS:

• All of the CONS of (2), plus:
• I'm unsure of how the value of waveformProperty needs to be created, and how/whether we can access its amplitude Properties in Studio.

Studio Tree:

Looks like this for numberOfHarmonicsProperty = 3:

-fourier-making-waves
 -discreteScreen
  -model
    numberOfHarmonicsProperty {NumberProperty}
    waveformProperty {Property.<Waveform>}
  -view
    -amplitudesPanel
      -amplitudeChart
        -amplitudeSliderGroup {PhetioGroup}
         +amplitude1Slider
         +amplitude2Slider
         +amplitude3Slider
    -harmonicsAccordionBox
      -harmonicsChart
       -harmonicPlotGroup {PhetioGroup}
         +harmonic1Plot
         +harmonic2Plot
         +harmonic3Plot
    -sumAccordionBox
      -sumChart
        +sumPlot

---

So... My preference is for approach (1). Once we choose an option, beware that changing will be VERY costly.

[pixelzoom]

In phetsims/tandem#226 (comment), @samreid also recommended approach (1):

For Fourier, I recommend creating amplitude names via string concatenation: amplitude${n}Property and creating them statically, not using PhetioGroup.

[samreid]

To elaborate, the instrumentation guide ( https://github.com/phetsims/phet-io/blob/master/doc/phet-io-instrumentation-technical-guide.md ) says:

Here is an ordered list of how to approach instrumenting an element that is dynamically created:

1. Does it even need instrumentation? Often instances don't need to be instrumented, or can perhaps be instrumented as
   a component of their parent (instead of being instrumented themselves)
2. Can it be created eagerly? Allocating dynamic elements on startup simplifies the instrumentation process, especially when supporting PhET-iO state and API validation.
3. Instrument the object dynamically, using PhetioGroup or PhetioCapsule. Use PhetioCapsule for single dynamic instances. Use PhetioGroup for multiple instances of the same type. If you have a use case that is not addressed by one or both of those, then please consult with the PhET-iO subteam, and potentially create a new IO Type suitable for your simulation.

It seems like the amplitude Properties fall under level 2.

[pixelzoom]

11/12/2020 design meeting: @arouinfar @kathy-phet @samreid @pixelzoom

Consensus was to go with approach (1).