implementation-notes.md issues

[pixelzoom]

Related to code review #259, where @samreid said:

I recommend to start with the documentation:
wave-interference/doc/implementation-notes.md
wave-interference/doc/model.md

There are some formatting problems, and I have a couple of specific questions.

[pixelzoom]

I fixed quite a bit of formatting in the above commits. Remaining issues for @samreid to address:

• The equations that begins f(x,y,t+1) is malformed, probably just missing a closing paren on the right side of the equation.
• As a courtesy to the reader, I've typically linked class names to their source files. E.g. WaveInterferenceQueryParameters. Up to you whether to do this.
• In "the observed Wavelength and Oscillation Time", why are "Wavelength" and "Oscillation Time" in caps?
• In "you can show the IdealInterferenceOverlay" what is "IdealInterferenceOverlay"? I thought it was a class name, but not found.

[samreid]

The equations that begins f(x,y,t+1) is malformed, probably just missing a closing paren on the right side of the equation.

I'm not seeing the problem. The current value is:

f(x,y,t+1) = c*c(f(x+1,y,t) + f(x-1,y,t) + f(x,y-1,t) + f(x,y+1,t) - 4*f(x,y,t)) - f(x,y,t-1) + 2*f(x,y,t)

Giving each function call a "nickname", this is equivalent to:

f(x,y,t+1) = c*c(f1 + f2 + f3 + f4 - 4*f5) - f6 + 2*f7

Can you please elaborate? I'm probably missing something.

[pixelzoom]

You're right, it's wellformed - I got lost in the parens.

[samreid]

In "the observed Wavelength and Oscillation Time", why are "Wavelength" and "Oscillation Time" in caps?

Fixed in aforementioned commit.

[samreid]

In "you can show the IdealInterferenceOverlay" what is "IdealInterferenceOverlay"? I thought it was a class name, but not found.

I fixed the type name, thanks!

As a courtesy to the reader, I've typically linked class names to their source files. E.g. WaveInterferenceQueryParameters. Up to you whether to do this.

Good idea! I added numerous links to source files.

[pixelzoom]

implementation-notes.md says:

There are 3 coordinate frames:

• view coordinates
• lattice coordinates (integer), with damping regions
• Scene-specific physical coordinates (such as cm or nm)

I had a difficult time locating the transforms related to these. How about some "see..." pointers for each?

[pixelzoom]

... for example, I see this in BarriersNode:

      // @private - Convert from model coordinates to view coordinates
      this.modelViewTransform = ModelViewTransform2.createRectangleMapping(
        this.scene.getWaveAreaBounds(),
        viewBounds
      );

      var latticeBounds = new Bounds2( 0, 0, 1, 1 );
      var modelBounds = scene.modelToLatticeTransform.viewToModelBounds( latticeBounds );
      var tempViewBounds = this.modelViewTransform.modelToViewBounds( modelBounds );

      //REVIEW missing visibility annotation
      this.latticeToViewTransform = ModelViewTransform2.createRectangleMapping( latticeBounds, tempViewBounds );

... and I suspect that these are what is being referred to in implementation-notes.md. But why are the transforms in BarrierNode, a view component that is specific to the Slits screen?

[samreid]

But why are the transforms in BarrierNode, a view component that is specific to the Slits screen?

latticeToViewTransform is only needed to shape and position the barriers, hence it is defined in BarrierNode. Anything else to do for this issue other than to add "see..." pointers for the coordinate frames in implementation-notes?

[pixelzoom]

Adding "see..." pointers should do it.

[samreid]

On hold until #292 is resolved.

[samreid]

I updated the docs as part of #292, @pixelzoom can you please review and advise?

[pixelzoom]

Description of coordinate frames and transforms looks good. I made some minor changes in the above commit.

Looks like all other points have been addressed, so feel free to close this if there's nothing else to do.

[samreid]

Thanks for the link and formatting, closing.