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

Feature/vector physics #139

Merged
merged 7 commits into from
Oct 12, 2020

Conversation

g-belmonte
Copy link
Contributor

Pull request related to #127

Pull request checklist:

  • Is the pull request from a dedicated branch in your fork instead of
    master?
  • Have you added yourself to the AUTHORS file? Not mandatory if you'd prefer
    to stay anonymous, but don't be shy!
  • Is "Allow edits from maintainers" enabled?
  • Is the code formatted using the same version of elm-format as the rest of
    elm-geometry?
  • Is code (mostly) wrapped to 80 columns?
  • BONUS POINTS: Have you added tests for new functionality?
  • EXTRA BONUS POINTS: Have you added documentation for new functionality?

There are still no tests and documentation. I wanted to check first if I'm going in the right direction :)

@g-belmonte g-belmonte changed the title Feature/vector physics WIP: Feature/vector physics Oct 10, 2020
@ianmackenzie
Copy link
Owner

I think for consistency with Vector3d.meters etc., the functions should probably have signatures like

Vector3d.metersPerSecond : Float -> Float -> Float -> Vector3d MetersPerSecond coordinates

I agree that the length-and-direction method is also useful, but being able to write

Vector3d.metersPerSecond 5 Direction3d.z

instead of

Vector3d.withLength (Speed.metersPerSecond 5) Direction3d.z

is not quite as much of a savings as being able to write

Vector3d.metersPerSecond 3 0 4

instead of what you'd currently have to do to get that:

Vector3d.xyz
    (Speed.metersPerSecond 3)
    (Speed.metersPerSecond 0)
    (Speed.metersPerSecond 4)

The three-argument form is also useful for JSON decoding, e.g.

Decode.map3 Vector3d.metersPerSecond
    (Decode.field "x" Decode.float)
    (Decode.field "y" Decode.float)
    (Decode.field "z" Decode.float)

@ianmackenzie
Copy link
Owner

I'm also not sure if it makes sense to talk about a pressure vector...I think pressure is pretty fundamentally a scalar quantity. Was there a particular application you were considering?

I think area vectors will come up in practice, but also I think those will usually end up being computed with code like

triangleAreaVector =
    Triangle3d.normalDirection triangle
        |> Maybe.map (Vector3d.withLength (Triangle3d.area triangle))
        |> Maybe.withDefault Vector3d.zero

instead of being constructed explicitly from their coordinates, so I don't think it would make sense to add Vector3d.squareMeters or anything like that.

@g-belmonte
Copy link
Contributor Author

Now I gotcha! I'll do some changes 😁

As for the pressure, yes, you're right. Pressure is always applied perpendicular to the surface. I'll remove those :)

Am I missing any other vector unit?

@g-belmonte
Copy link
Contributor Author

Changes done :)

About those area vectors, Vector3d.squareMeters sounds a bit weird to me, since area is an scalar 😅
But using the normal of a surface is pretty common.
Wdyt, should I implement it too?

@ianmackenzie
Copy link
Owner

I'd leave out area for now. Area vectors do come up and are useful mathematically - for example, if you multiply the area vector of a triangle (as computed above) by a scalar pressure value, then the product gives you a force value which is equal to the force vector exerted on that triangle by the given pressure! But again, I don't really see a use case for directly constructing area vectors from their XYZ components, so I would say leave them out.

I think the code is looking great now! I can't think of any other units to add (I did a general review of the units type in elm-units and nothing jumped out at me - and I think speed, acceleration and force will certainly be the main ones when working with elm-physics).

I'm happy to write docs if you'd like, but if you do want to take a crack then I think the best approach would be to have a single documentation paragraph at the top of the 'Physics' section in each of the two modules that describes the general idea and has one or two examples - I certainly don't think you need a separate description/example for each function, they're pretty self-explanatory.

@g-belmonte
Copy link
Contributor Author

I'm happy to be of assistance! 😀

I was actually going to ask you for help with the docs. I've been playing around with elm-geometry and elm-units for a pair of weeks, so I can't really think of any meaningful examples or describing a good use case at the moment 😅
Could you take care of the docs for me then?

Also, wdyt about testing? Do you prefer to have everything tested or just a few key things? If the latter, then do you see any valuable tests I could write?

@ianmackenzie
Copy link
Owner

OK just added some simple docs - see what you think!

Honestly not sure what tests would be really meaningful for these - I took a look through to check for any typos and it all looked good, so if you're happy with this then I think it's ready to merge.

@g-belmonte
Copy link
Contributor Author

The docs looks good :)
I also couldn't think of any meaningful test, so I think it is good to go!

@g-belmonte g-belmonte changed the title WIP: Feature/vector physics Feature/vector physics Oct 12, 2020
@ianmackenzie ianmackenzie merged commit 8d64b0d into ianmackenzie:master Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants