Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

model.md #134

Closed
Tracked by #129
veillette opened this issue Jul 27, 2021 · 9 comments
Closed
Tracked by #129

model.md #134

veillette opened this issue Jul 27, 2021 · 9 comments
Assignees
Labels
documentation Improvements or additions to documentation status:ready-for-review

Comments

@veillette
Copy link
Contributor

veillette commented Jul 27, 2021

From the CRC:

  • Does model.md adequately describe the model, in terms appropriate for teachers?

See https://github.com/phetsims/geometric-optics/blob/master/doc/model.md

@veillette
Copy link
Contributor Author

Duplicate of #36

@veillette veillette marked this as a duplicate of #36 Jul 27, 2021
veillette added a commit that referenced this issue Jul 30, 2021
@veillette
Copy link
Contributor Author

@arouinfar : I took a stab at writing a draft of the model but you are in a better position to write it.
Assigning to @arouinfar .

@veillette veillette mentioned this issue Jul 30, 2021
@veillette veillette added the documentation Improvements or additions to documentation label Jul 31, 2021
@zepumph
Copy link
Member

zepumph commented Aug 4, 2021

Note some formatting done in 276d32b

@pixelzoom
Copy link
Contributor

@arouinfar Let's hold off on this review until we're closer to publication, in case I need to make any model changes.

@pixelzoom pixelzoom changed the title Does model.md adequately describe the model, in terms appropriate for teachers? model.md Sep 22, 2021
@pixelzoom
Copy link
Contributor

pixelzoom commented Sep 22, 2021

Reviewing model.md and implementation-notes.md... I would recommend adding a Terminology section to model.md, and moving the general terminology from implementation-notes.md to model.md. Otherwise these files are looking pretty good.

@pixelzoom
Copy link
Contributor

@arouinfar ready for review. If you'd like to modify, feel free to do so directly.

This must be reviewed before code review can begin.

arouinfar added a commit that referenced this issue Mar 22, 2022
@arouinfar
Copy link
Contributor

@pixelzoom this was really nicely done. I fixed a few small things in the commit above.

The last sentence in this section is a bit unclear, and I recommend re-wording it. In particular, I'm not sure what you meant by, "The light spots... are based on the size of the light spot."

Light sources: Light sources can be projected onto a screen. The light spots that appear on the screen are based on the aperture of the lens (i.e. its diameter) and the size of the light spot.

@pixelzoom
Copy link
Contributor

Thanks @arouinfar. I must give credit to @veillette. Most of the stuff below the Abbreviations section was his work. I reorganized it into sections, and did a little worksmithing.

The last sentence you referred to has been modified to read:

Light sources: Light sources can be projected onto a screen. Brightness of the light spots on the screen is based on the diameter of the lens and the diameter of the light spot.

If this makes sense, please close.

@arouinfar
Copy link
Contributor

Looks good, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation status:ready-for-review
Projects
None yet
Development

No branches or pull requests

4 participants