phetsims / faradays-electromagnetic-lab

"Faraday's Electromagnetic Lab" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
1 stars 0 forks source link

Investigate why `lightsWhenCurrentChangesDirection` is needed. #152

Closed pixelzoom closed 7 months ago

pixelzoom commented 7 months ago

For #103 code review. In https://github.com/phetsims/faradays-electromagnetic-lab/issues/122, this REVIEW comment was added to LightBulb.ts:

  // Determines whether the bulb lights when the current in the coil changes direction.
  // In some cases (e.g. flipping the bar magnet) this should be true.
  // In other cases (eg, the Generator or AC Electromagnet) this should be false.
+
+ // REVIEW - Can you explain more about the difference between these two cases? Why is this needed, and why does the designed behavior require this distinction?
  lightsWhenCurrentChangesDirection?: boolean;

This was inherited from the Java version, specifically private boolean _offWhenCurrentChangesDirection in LightBulb.java. It was ported directly to TypeScript, then the logic was inverted and it was renamed lightsWhenCurrentChangesDirection.

Unfortunately there's no additional documentation. And while I (dimly) recall that this was added to address a problem, I can't find an Unfuddle issue that refers to this.

pixelzoom commented 7 months ago

Poking throught the Unfuddle SVN history for LightBulb.java, I determined that this feature was added in https://phet.unfuddle.com/a#/projects/9404/repositories/23262/commit?commit=3111, with this commit message:

Lightbulb can optionally go "off" when direction of current in the pickup coil changes. This is the desired behavior for the Generator and Transformer, since they have sinusoidal B-field sources.

The setOffWhenCurrentChangesDirection method that was added to LightBulb.java included some documentation:

    /**
     * Determines whether the lightbulb turns off when the current in the coil
     * changes direction.  In some cases (eg, the Generator or AC Electromagnet)
     * this is the desired behavoir.  In other cases (eg, polarity file of the 
     * Bar Magnet) this is not the desired behavior.
     * 
     * @param offWhenCurrentChangesDirection true or false
     */
    public void setOffWhenCurrentChangesDirection( boolean offWhenCurrentChangesDirection ) {
        _offWhenCurrentChangesDirection = offWhenCurrentChangesDirection;
    }

That's basically what the TypeScript documentation says. And I'm not clear on why this is the desired behavior, so I'll need to inquire/investigate before I can improve the doc.

pixelzoom commented 7 months ago

The SVN commit message said "This is the desired behavior for the Generator and Transformer, since they have sinusoidal B-field sources." But we're setting lightsWhenCurrentChangesDirection: false for the Transformer screen, regardless of whether the AC or DC power supply is selected. So after wondering why this behavior is desired for sinusoidal B-field sources, I'm also wondering if this is appropriate for the DC power supply, which is not sinusoidal.

pixelzoom commented 7 months ago

More clues are in the TODO.txt file that was updated in https://phet.unfuddle.com/a#/projects/9404/repositories/23262/commit?commit=3111:

DONE:
v0r25...
+ (4/24) - lightbulb can optionally go "off" when direction of current in the pickup coil changes

NO:
+ (4/24) - calculate periods of Generator and AC so that delta flux is zero/max at correct point (no, handle this in Lightbulb)

So it looks like this feature in LightBulb was added to address a problem with the Generator and AC power supply at its zero and max voltage points. I guess I could set lightsWhenCurrentChangesDirection: true for those cases and see what problem(s) that causes.

pixelzoom commented 7 months ago

This feature was added in the Java version to address the fact that the model may step over zero points for current that is cyclic -- namely, the AC Power Supply and Generator. Zero points are where the current's direction changes. And we want to guaranteee that the light bulb displays an 'off' state. This is hard to see when the sim is running, but easy to see if you pause the sim and step it with the time controls.

The doc was lousy, so I improved it. In LightBulb.ts:

  // For current that is cyclic and changes direction (AC Electromagnet, Generator) the model may step over the
  // zero points, which is where the current changes dirction. This Property should be set to false for those cases,
  // so that the light bulb is guaranteed to go through an 'off' state. This is most noticeable when stepping the sim
  // manually using the time controls. See https://github.com/phetsims/faradays-electromagnetic-lab/issues/152.
  lightsWhenCurrentChangesDirectionProperty?: TReadOnlyProperty<boolean>;

The implementation incorrectly set lightsWhenCurrentChangesDirection: true for the DC power supply (battery), so I also fixed that. In Transformer.ts:

        // AC Power Supply is cyclic, and current changes direction. We might 'step over' the zero points in the model,
        // so ensure that the light bulb will go through an 'off' state as the current direction changes. Note that it
        // is still appropriate for the light bulb to light when the current changes direction with the DC Power Supply.
        // See https://github.com/phetsims/faradays-electromagnetic-lab/issues/152.
        lightsWhenCurrentChangesDirectionProperty: new DerivedProperty( [ this.electromagnet.currentSourceProperty ],
          currentSource => currentSource instanceof DCPowerSupply )

Closing.