Store moment calculated from CP and CNa instead of CP itself #2753
Conversation
…hould send an AERODYNAMIC_CHANGE (instead of a BOTH_CHANGE for all three)
… is overridden by an ancestor, compute as needed
… by CP * CNa. When mergine AerodynamicForces, this lets us just add the moments instead of doing a weight average of the CPs; this in turn avoids a divide-by-zero error caused by nose cones having a weight of 2 and tail cones having a weight of -2 when accumulating CP for a rocket. setCP() still expects a Coordinate consisting of the CP with a weight defined by CNa, and getCP() still returns a similar Coordinate. When setCP() is given a CP with a weight of 0, the x, y, z values are replaced by 0. When getCP() is called and the weight is 0, the CP values returned are 0 (it would arguably be more correct to return NaN, that could be a confusing result for really tiny fins).
… 0. This rotates it so we have a weight of 1.
setCNa() calls go away completely, (CNa is set by passing the weight in with the CP coordinate), and getCNa() is replaced by getCP().weight
|
Outstanding. |
|
@JoePfeiffer in the following image, I compare the component analysis of a simple model rocket of your PR (left) with the unstable branch (right), and the CP for the body tube and launch lug is 0 cm. I suppose this is not desired? |
|
Also: this PR makes slight modifications to Sampo's original thesis calculations, right? If so, we should eventually update the technical docs. I won't ask you to do update the LaTeX doc in this PR, but can you please add a txt document in |
|
The change in CP for the body tube doesn't matter, since its weight (CNa) is 0. The change for the launch lug... I need to see if the CNa should really be 0. If so it also doesn't matter, if not there's an issue elsewhere. I'll make this a draft until I can check that. This should be mathematically equivalent to the old code, except for finessing the divide-by-zero. Looking at equation 3.29 in the thesis, this is actually a more direct implementation of the equation. The numerator is the sum of the moments, and the denominator is the sum of the weights. The old code implemented this with a bunch of weighted averages; the new code accumulates the numerator in the moment and the denominator in the weight, and then does the division when getCP() is called. |
|
I don't know why the launch lug was ever reporting a non-zero CP; the calculateNonAxialForces() method in LaunchLugCalc is empty. The launch lug's CP is also being reported (before this PR) several centimeters forward of the actual launch lug. |
|
Does this PR also fix #2291? |
The discontinuity observed in #2751 when a component has a tailcone is caused by a divide-by-zero error when merging CPs for the components in the rocket.
At present, AerodynamicForces contains a member variable cp containing the center of pressure of the component or assembly, with CNa as its weight. It also redundantly stores CNa as a separate variable.
The discontinuity when there is a tailcone comes from the fact that, according to the Barrowman equations, a nose cone has a CNa of 2 and a tail cone has a CNa of -2. When taking the weighted average of the nose and tail cone the weight becomes 0; when dividing by the weight the CP is NaN.
This PR solves this by storing the moment defined by CP * CNa, with a weight of CNa. Now instead of combining CPs by performing a weighted average, we can just sum the moments. When getCP() is called, the moment is divided by the weight to return the CP. If the weight happens to be 0 at this point, a CP of (0, 0, 0) is returned (it doesn't return (NaN, NaN, NaN) because this could cause issues with things like really tiny fins).
It also gets rid of the separate storage of CNa.
Fixes #2751