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 #263

Closed
pixelzoom opened this issue Mar 8, 2023 · 10 comments
Closed

model.md #263

pixelzoom opened this issue Mar 8, 2023 · 10 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Mar 8, 2023

For #262

Link: https://github.com/phetsims/calculus-grapher/blob/master/doc/model.md

We'll need to complete this before publication, because it's linked into the PhET-iO materials that are delivered to clients.

It would be nice to have before official code review, but not essential.

This was referenced Mar 8, 2023
veillette added a commit that referenced this issue Mar 9, 2023
veillette added a commit that referenced this issue Mar 10, 2023
veillette added a commit that referenced this issue Mar 16, 2023
Signed-off-by: Martin Veillette <[email protected]>
@veillette veillette self-assigned this Mar 16, 2023
veillette added a commit that referenced this issue Mar 18, 2023
Signed-off-by: Martin Veillette <[email protected]>
pixelzoom added a commit that referenced this issue Mar 19, 2023
pixelzoom added a commit that referenced this issue Mar 19, 2023
pixelzoom added a commit that referenced this issue Mar 19, 2023
@pixelzoom
Copy link
Contributor Author

I made a couple changes for proof reading in the above commits.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Mar 20, 2023

Some general feedback on model.md... Currently, this file reads like an overview of the sim's design and features. While well-written, that's generally not what this file is about. It's supposed give teachers and PhET-iO clients an overview that helps them understand the underlying model.

Here are the things that I think need to be added:

  • Curves are approximated via a set of discrete points with fixed x coordinates. Each coordinate has meta data ("pointType").
  • Derived curves are computed from f(x), and especially why you did that for second derivative.
  • Describe cusps and how they are used to indentify discontinuities.
  • Anything that you think is significant related to how you computed the integral, derivative, and second derivate. For example. you said "A finite difference method is used to determine the derivative". Expand on that a bit, and do the same for the other derived curves.
  • High-level description of how curve manipulation is handled, how that relates to undo, etc.
  • High-level description of how undo is handled - when points are saved, undo history is finite, etc.

@pixelzoom
Copy link
Contributor Author

@veillette after you're happy with model.md, the next step is to assign to @amanda-phet for review. The sim design typically signs off on model.md, once they are satisfied that it's useful for the target audience (teachers, PhET-iO clients). The designer may also ask you to add things that they feel are important.

veillette added a commit that referenced this issue Mar 20, 2023
@veillette
Copy link
Contributor

veillette commented Mar 20, 2023

@pixelzoom : I added a section called "## Approximations, Limitations and Implementation Details" where I explained some internal details of our implementations without going too much into the weeds. It does include all the elements from your comment (#263 (comment). )

@veillette
Copy link
Contributor

veillette commented Mar 20, 2023

One aspect that is not included is a description of the mathematical transformation associated with each curve manipulation mode. Since there are many of them, it will be a long passage, but maybe that should be included. @amanda-phet, what do you think? Should I add such a section ?

@veillette
Copy link
Contributor

@amanda-phet: Should we add such a section about that describes each curve manipulation mode?

@veillette veillette assigned amanda-phet and unassigned pixelzoom and veillette Mar 23, 2023
veillette added a commit that referenced this issue Mar 24, 2023
Signed-off-by: Martin Veillette <[email protected]>
veillette added a commit that referenced this issue Mar 24, 2023
Signed-off-by: Martin Veillette <[email protected]>
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Mar 24, 2023

@veillette and I discussed. I think it would be useful to have a section that briefly enumerates the curve manipulation modes, and includes:

  • their names (hill, pedestal, ...), for correlation with tandem names
  • identify which ones are relevant for "width"
  • a brief description of the curve that they draw

@veillette veillette assigned veillette and unassigned amanda-phet Mar 24, 2023
veillette added a commit that referenced this issue Mar 24, 2023
veillette added a commit that referenced this issue Mar 24, 2023
Signed-off-by: Martin Veillette <[email protected]>
@veillette
Copy link
Contributor

I added a section of the curve manipulation modes. I am reassigning to @amanda-phet to review and make any additions you judge necessary.

@veillette veillette assigned amanda-phet and unassigned veillette Mar 24, 2023
veillette added a commit that referenced this issue Mar 27, 2023
veillette added a commit that referenced this issue Mar 27, 2023
Signed-off-by: Martin Veillette <[email protected]>
amanda-phet added a commit that referenced this issue Mar 27, 2023
@amanda-phet
Copy link
Contributor

I made three edits. I'm not sure if I worded the 3rd one correct, but it had a typo (mentioning tangent line in the area under the curve section).

@veillette please review and if it looks good I think this can be closed.

@amanda-phet amanda-phet assigned veillette and unassigned amanda-phet Mar 27, 2023
@veillette
Copy link
Contributor

Good catch @amanda-phet ! I think we can close then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants