phetsims / geometric-optics

Geometric Optics is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
6 stars 5 forks source link

Index of refraction n=2 for mirrors #452

Closed veillette closed 1 year ago

veillette commented 1 year ago

In studio, we see that the mirror has an index of refraction set to 2.

This is noted in implementation-notes it says

Physical mirrors do not have an index of refraction. Our mirror is modeled as a lens with index of refraction = 2. See INDEX_OF_REFRACTION_RANGE in Mirror.ts.

Physically, mirrors do not have an index of refraction.

For lenses, the focal length is given by the lens-maker's equation, $$f{lens} (n) = -\frac{R}{2 (1-n) }.$$ For mirrors, the focal length is determined by the paraxial approximation, given by $$f{mirror} = \frac{R}{2} ,$$ then mathematically, we have $$f{mirror} = f{lens} (n=2). $$ Therefore, from that point of view, the focal length of a mirror is equivalent to a lens with a index of refraction of 2.

In essence, it is just a simple, convenient mathematical equivalence. However, with phet-io, we are now exposing this simple little hack to instructional designers. If you have never thought about it, it is confusing to think of a mirror as a lens an index of refraction of 2.

veillette commented 1 year ago

There are different ways to go about it:

arouinfar commented 1 year ago

Good find @veillette. Using n=2 for mirrors is mathematically convenient (albeit physically nonsensical). This seems like an implementation detail that we don't need to expose to the clients. My recommendation would be to uninstrument mirror.indexOfRefractionProperty.

veillette commented 1 year ago

One minimally invasive way to stop the instrumentation for mirror is to do the following in Optic.js

    // Get the IOR value from the current focal-length model.
    this.indexOfRefractionProperty = new DerivedProperty(
      [ GOPreferences.focalLengthModelTypeProperty, this.directFocalLengthModel.indexOfRefractionProperty,
        this.indirectFocalLengthModel.indexOfRefractionProperty ],
      ( focalLengthModelType, directIndexOfRefraction, indirectIndexOfRefraction ) =>
        ( focalLengthModelType === 'direct' ) ? directIndexOfRefraction : indirectIndexOfRefraction, {
        // units: unitless
        tandem: ( options.tandem.name === 'mirror' ) ? Tandem.OPT_OUT : options.tandem.createTandem( 'indexOfRefractionProperty' ),
        phetioDocumentation: 'The index of refraction (IOR) of the optic',
        phetioValueType: NumberIO
      } );

However, the conditional ( options.tandem.name === 'mirror' ) is little bit brittle, (but maybe ok since it only affects studio).

Ideally we would like something more robust for our conditional.

   tandem: ( this instanceOf Mirror ) ? Tandem.OPT_OUT : options.tandem.createTandem( 'indexOfRefractionProperty' ),

but this bring some circular dependencies between Mirror which is extending Optics and Optics that references Mirror

The other options is to have Mirror bring a flag isMirror=true into optics.

At @pixelzoom would probably have an informed opinion about it.

pixelzoom commented 1 year ago

Using n=2 for the IOR of mirrors is not the only case that is not physically correct. The flat mirror is modeled as a convex mirror with a very large focal length (100,000 cm), when the focal length should be infinite.

Both of these model simplifications are well-documented in model.md, which is delivered with PhET-iO:

The mirror is modeled as a lens with index of refraction = 2. The flat mirror is modeled as a convex mirror with a very large focal length (100,000 cm).

Under the hood, it's not always the case that PhET models are (or can be) physically correct. And sometimes they are so far from being physically-correct that we refer to them as "Hollywooded". Rather than hide that fact from PhET-iO clients, we should be doing the opposite -- making them very aware of how things are implemented, and how that might translate to things that they need to consider.

For mirrrors specifically, we previously discussed how to proceed during implementation. For example, we considered using f=infinity for flat mirrors, which would have resulted in a much more complicated implementation. We concluded that we preferred to simplify the implementation, and document in model.md. I don't think we should endeavor to hide this fact via conditional PhET-iO instrumentation. We could elaborate in the phetioDocumentation for the relevant mirror Properties, but I feel that's unnecessary because it's well-documented in model.md.

So... Unless our previous conclusions are no longer appropriate, my recommendation is to do nothing and close this issue.

pixelzoom commented 1 year ago

In https://github.com/phetsims/geometric-optics/issues/452#issuecomment-1512064496, @arouinfar said:

... My recommendation would be to uninstrument mirror.indexOfRefractionProperty.

Deciding what to uninstrument will involve more discussion. You'd also presumably want to look at uninstrumenting mirror.focalLengthModels.directFocalLengthModel.indexOfRefractionProperty and mirror.focalLengthModels.indirectFocalLengthModel.indexOfRefractionProperty. And what about mirror.focalLengthProperty and its dependencies?

Imo, uninstrumenting to address this issue adds a lot of complexity to the implementation (in the form of conditional instrumentation) for little-to-no value.

pixelzoom commented 1 year ago

So... My recommendation (see preceeding 2 comments) is to do nothing for this issue. Assigning to @arouinfar to decide how to proceed.

arouinfar commented 1 year ago

You bring up a lot of excellent points @pixelzoom, let's close. We've already unfeatured mirror.indexOfRefractionProperty because it is less useful to instructional designers.