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

Code Review: Add model.md #96

Closed
AgustinVallejo opened this issue Mar 9, 2023 · 10 comments
Closed

Code Review: Add model.md #96

AgustinVallejo opened this issue Mar 9, 2023 · 10 comments

Comments

@AgustinVallejo
Copy link
Contributor

For #88

@jonathanolson
Copy link
Contributor

Assign me once this is done for review (I'll also be available to help create it, if so that won't be needed)

@samreid
Copy link
Member

samreid commented Mar 16, 2023

@AgustinVallejo said this is ready for review. @jonathanolson can you please take a look?

@AgustinVallejo
Copy link
Contributor Author

I believe this, along #97 are ready for review

@jonathanolson
Copy link
Contributor

I think it would be good to put the Lab mode configurations into the model.md, so that people can see what coordinates they start with (without being rounded at all). Since we're setting them in "model" units, they would be getting rounded "view" units.

@jonathanolson
Copy link
Contributor

Oops, actually, they're referenced! We'll need to not break that link.

@jonathanolson
Copy link
Contributor

Made some changes above, can you review?

Additionally, I think it would be good to mention some things about collisions. Does it preserve momentum?

@jonathanolson
Copy link
Contributor

Looks good, just the one item above!

@AgustinVallejo
Copy link
Contributor Author

Added a big section with Physical Simplifications, to adress body sizes, mass scales, collisions, and how accurate is each pre-set with their real world counterparts. So another fast review would be nice.

@jonathanolson
Copy link
Contributor

This is excellent!

My only question would be whether the "Distances and Sizes" and "Radii of Bodies" are really separate - they are both talking about the radii/size (same thing?), and might be combinable.

@AgustinVallejo
Copy link
Contributor Author

AgustinVallejo commented Mar 20, 2023

I think they work good separately, because one section is stating that body sizes are actually way bigger than in reality, the other one states that we are using a particular mapping function between mass and radii. Closing.

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