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

[Review] Start on Projectile SRS #1428

Merged
merged 153 commits into from
Jun 27, 2019
Merged

[Review] Start on Projectile SRS #1428

merged 153 commits into from
Jun 27, 2019

Conversation

samm82
Copy link
Collaborator

@samm82 samm82 commented May 28, 2019

So far, I added the Goal Statements, Assumptions, TMs, DDs, GD, and IMs, and I think this is enough to review. In particular, I'm not 100% if the concepts/unitals are defined clearly enough/if I'm using the default physics concepts too heavily. Also, I think technically the velocity entered is speed since it is a scalar with units (m/s), but since an angle is specified as well, I think it might be OK?

Closes #1430, closes #1431, closes #1493, closes #1577, closes #1584, closes #1617, closes #1620, closes #1621, closes #1622, and closes #1623

Copy link
Collaborator

@bmaclach bmaclach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thinks this looks good. I don't think you are using the default physics concepts too heavily - I would even say that some of the Unitals you define in the projectile example belong in drasil-data (initial and final velocity, x and y components of velocity, these are very general and could apply to many examples).

There's definitely some inconsistency with regards to quantities being vector or scalar. In the final velocity GD, for example, the equation is wrong because somehow the acceleration vector is turning into a scalar. I think final and initial velocity should both be defined as vectors? With the Distance GD as well everything (except time) should be a vector. But then elsewhere initial velocity is used as a scalar (i.e. in the Distance (refined) GD). Probably this means we should have two unitals: one for the initial velocity vector and one for the initial speed scalar. In the Air Time and Distance (refined) GDs, the denominator should really be the magnitude of the acceleration vector. There's also vector/scalar mismatch in the DDs. v_x is really the magnitude of v times cos(theta), and similarly for v_y.

Some other minor comments:

  • instead of "angle of projectile", I think the description for theta should be "angle at which projectile is launched" or simply "launch angle"
  • d_offset is unitless but based on the equation for it, it should have units of metres

@samm82

This comment has been minimized.

@bmaclach

This comment has been minimized.

@samm82 samm82 mentioned this pull request Jun 26, 2019
4 tasks
@samm82
Copy link
Collaborator Author

samm82 commented Jun 27, 2019

Barring any unforeseen issues, Projectile is ready to be launched! (Pun very much intended) @smiths @JacquesCarette

@samm82
Copy link
Collaborator Author

samm82 commented Jun 27, 2019

@smiths Other than the Hibbeler reference - I totally forgot to ask in the meeting! Would I be able to grab the book today?

@smiths
Copy link
Collaborator

smiths commented Jun 27, 2019

I just sent you an e-mail about the book. 😄 Please come by my office to pick it up.

Copy link
Collaborator

@smiths smiths left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are in a good spot to merge Projectile in. We have a good initial draft. We can make future changes with this as a starting point.

@smiths smiths merged commit af2acf8 into master Jun 27, 2019
@smiths smiths deleted the projectileSRS branch June 27, 2019 21:54
@smiths
Copy link
Collaborator

smiths commented Jun 27, 2019

Since @JacquesCarette will not be able to review this for at least a week, and possibly longer, I've merged it into master. We can create new branches as we continue to develop certain aspects of Projectile, such as the code generation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
5 participants