-
Notifications
You must be signed in to change notification settings - Fork 69
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
Propagate the dynamical elements from the parts to the pile-up #2397
Conversation
@@ -71,8 +67,7 @@ void Subset<Part>::Properties::Collect( | |||
collected_ = true; | |||
if (EqualsExistingPileUp()) { | |||
PileUp& pile_up = *parts_.front()->containing_pile_up(); | |||
pile_up.set_mass(total_mass_); | |||
pile_up.set_intrinsic_force(total_intrinsic_force_); | |||
pile_up.RecomputeFromParts(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aren't you making things quadratic in the number of parts instead of linear here? Part counts can get pretty big, and this runs at every frame; we should be careful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, this happens once when the pile-up is constructed/updated, so O(N).
ksp_plugin/pile_up.cpp
Outdated
|
||
Bivector<AngularMomentum, RigidPart> const part_angular_momentum = | ||
Anticommutator(part_inertia_tensor, | ||
part_rigid_motion.angular_velocity_of_to_frame()); | ||
part_to_barycentric.angular_velocity_of_to_frame()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
part_to_barycentric.angular_velocity_of_to_frame
is the angular velocity of Barycentric
; that feels wrong somehow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to part_to_pile_up
which is more logical but not different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But still, this is the angular velocity of the pile up axes (i.e., of non rotating axes) in the axes of the part, rather than the angular velocity of the part as seen in non-rotating axes; I think that is a sign error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed by switching everything to RigidPileUp
.
@@ -424,27 +426,40 @@ DegreesOfFreedom<Barycentric> PileUp::RecomputeFromParts( | |||
// Then compute the inertia tensor and the angular momentum of the pile-up. | |||
InertiaTensor<RigidPileUp> pile_up_inertia_tensor; | |||
Bivector<AngularMomentum, RigidPileUp> pile_up_angular_momentum; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using this type to compute the angular momentum only makes sense if RigidPileUp
remains non-rotating, as it is in non-rotating frames that the quantity ∫ r ∧ v dm is conserved (and is called the angular momentum).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But RigidPileUp
is non-rotating. I have fixed the comment in frames.hpp
.
Wedge(part_degrees_of_freedom.position() - RigidPileUp::origin, | ||
part->inertia_tensor().mass() * | ||
part_degrees_of_freedom.velocity()) * | ||
Radian; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please open an issue to track moving the computation of the momenta from the motion and the inertia to some place that can be unit-tested.
Not used much at this point. Tested (a bit) in game.