Triple counting of RCS torque
lamont-granquist opened this issue ยท 6 comments
I created a debugging patch kinda like this:
@@ -768,6 +786,7 @@ namespace MuMech
if (rw != null)
{
torqueAvailable += rw.GetPotentialTorque().Abs();
+ Debug.Log(" accumulated torqueAvailable after a ModuleReactionWheel: " + torqueAvailable);^M
}
else if (pm is ModuleEngines)
{
@@ -823,6 +842,7 @@ namespace MuMech
var crtlTorque = tp.GetPotentialTorque().Abs();
torqueAvailable += crtlTorque;
+ Debug.Log(" accumulated torqueAvailable after an ITorqueProvider: " + torqueAvailable);^M
}
for (int index = 0; index < vesselStatePartModuleExtensions.Count; index++)
@@ -854,12 +874,15 @@ namespace MuMech
// (Mathf.Abs(ctrlTorqueAvailablePos.y) + Mathf.Abs(ctrlTorqueAvailableNeg.y)) / 2f,
// (Mathf.Abs(ctrlTorqueAvailablePos.z) + Mathf.Abs(ctrlTorqueAvailableNeg.z)) / 2f);
torqueAvailable += torqueSurfStock;
+ Debug.Log(" accumulated torqueAvailable after torqueSurfStock: " + torqueAvailable);^M
// Max since most of the code only use positive control input and the min would be 0
torqueAvailable += Vector3d.Max(rcsTorqueAvailable.positive, rcsTorqueAvailable.negative);
+ Debug.Log(" accumulated torqueAvailable after rcsTorqueAvailable: " + torqueAvailable);^M
//torqueAvailable += Vector3d.Max(einfo.torqueEngineAvailable.positive, einfo.torqueEngineAvailable.negative);
torqueAvailable += torqueGimbalStock;
+ Debug.Log(" accumulated torqueAvailable after torqueGimbalStock: " + torqueAvailable);^M
// TODO : bring DifferentialThrottle back
//torqueFromDiffThrottle = Vector3d.Max(einfo.torqueDiffThrottle.positive, einfo.torqueDiffThrottle.negative);
Which outputs this on my test vehicle:
[LOG 18:11:59.727] accumulated torqueAvailable after an ITorqueProvider: [0, 0, 0]
[LOG 18:11:59.727] accumulated torqueAvailable after an ITorqueProvider: [0, 0, 0]
[LOG 18:11:59.727] accumulated torqueAvailable after an ITorqueProvider: [0, 0, 0]
[LOG 18:11:59.727] accumulated torqueAvailable after an ITorqueProvider: [1.50874257087708E-07, 0.0642891228199005, 0.115039341151714]
[LOG 18:11:59.727] accumulated torqueAvailable after an ITorqueProvider: [3.5017728805542E-07, 0.128761865198612, 0.231028899550438]
[LOG 18:11:59.727] accumulated torqueAvailable after an ITorqueProvider: [0.0657646805047989, 0.128762040287256, 0.231446523219347]
[LOG 18:11:59.727] accumulated torqueAvailable after an ITorqueProvider: [0.180804416537285, 0.193050991743803, 0.23144652962219]
[LOG 18:11:59.727] accumulated torqueAvailable after an ITorqueProvider: [0.296793766319752, 0.257523905485868, 0.231446557212621]
[LOG 18:11:59.727] accumulated torqueAvailable after an ITorqueProvider: [0.297211094992235, 0.257523923180997, 0.297212265897542]
[LOG 18:11:59.727] accumulated torqueAvailable after an ITorqueProvider: [0.297211296157911, 0.321812598966062, 0.412252098787576]
[LOG 18:11:59.727] accumulated torqueAvailable after an ITorqueProvider: [0.297211497323588, 0.386285408399999, 0.528241754043847]
[LOG 18:11:59.727] accumulated torqueAvailable after an ITorqueProvider: [0.362975827651098, 0.386285598389804, 0.528658666647971]
[LOG 18:11:59.727] accumulated torqueAvailable after an ITorqueProvider: [0.478015235858038, 0.450574266724288, 0.528658676892519]
[LOG 18:11:59.727] accumulated torqueAvailable after an ITorqueProvider: [0.594005069928244, 0.515046673826873, 0.528658699244261]
[LOG 18:11:59.727] accumulated torqueAvailable after an ITorqueProvider: [0.594422388123348, 0.515046692453325, 0.594423603266478]
[LOG 18:11:59.727] accumulated torqueAvailable after torqueSurfStock: [0.594422388123348, 0.515046692453325, 0.594423603266478]
[LOG 18:11:59.727] accumulated torqueAvailable after rcsTorqueAvailable: [0.891340576810762, 0.772937759757042, 0.891497589414939]
[LOG 18:11:59.727] accumulated torqueAvailable after torqueGimbalStock: [0.891340576810762, 0.772937759757042, 0.891497589414939]
Surface and gimbal are zero which is good because i'm in space without a gimballing engine active...
rcsTorqueAvailable is likely correct. i instrumented the rcsTorqueAvailable computation loop with debugs pretty extensively and i'm getting:
accumulated rcsTorqueAvailable: [ f: 0.297073986148462, b: 0.296433387324214, l: 0.296918188687414, r: 0.296587977092713, u: 0.257891067303717, d: 0.257155602797866 ]
Those are the kind of numbers i want based on hand calculation, including step-by-step going through the cal MJ does for several of the thrusters, so the torqueAvailable should be [ 0.296918188687414, 0.257891067303717, 0.296918188687414 ] which is what you get if you diff the "after rcsTroqueAvailable" line with the accumulated numbers from the ITorqueProvider's.
I also think this is no mistake that the numbers are off by a factor of 3.
There's two bugs here, one in MJ and one in stock.
- The MJ bug is that we're double counting the contribution from stock's ITorqueProvider and from its own calculation. Needs to be one or the other. That is fairly simple and i think for right now ITorqueProvider probably needs to lose.
- The stock bug is that the values are always positive and always additive so opposing thrusters add up when facing left or right and contribute to both positive and negative torque so stock tends to double-count.
Gory details from a single loop over my vesselparts is here:
https://gist.github.com/lamont-granquist/1f9ff6ec407945864899938b12edce22
Note for example:
[LOG 18:11:59.727] stock GetPotentialTorque(): (0.0004, 0.0000, 0.0658)
[LOG 18:11:59.727] stock GetPotentialTorque().Abs(): (0.0004, 0.0000, 0.0658)
[LOG 18:11:59.727] thruster torque(vessel): (0.0003, 0.0000, -0.0656)
Stock should really be returning that minus sign, and its not. And then to use the value MJ would still need to sum over those using a Vector6.
I now pushed all my changes. VesselState is stable on my side.
I cleaned up things, converted all torque to Vector6 and changed the infoItem to display the contribution of each sources.
The patch was rather large since I had more code that was not committed than I thought...
And I've hit my timebox of investigating this tonight. I'm lacking the bigger picture about what to do about this and what the ITorqueProvider stuff is doing there.
I suspect there needs to be a toggle to have MJ either sum with ITorqueProvider or else sum up RWs, RCS, Surfaces + Gimbals itself...
Its probably double-counting surface torque and gimbal torque as well?
I think reaction wheel torque happens earlier in the same loop over parts that counts up the ITorqueProviders, so those may not be double-counted...
@sarbian in think you've got the write-lock on re-writing VesselState right now, so I'll hold off on trying to write any code around this...
Yep, this is actually fixed by 28af35f
I guess we could close this, but it'd be kind of nice to do something better around it -- properly count up the torque the MJ way and then count up the ITorque contributions from the different kinds of parts and display them in a debug menu...
Quick reply before I do something else : ITorqueProvider is here to catch mods that provide torque. The main use is FAR control surface.
I will try to clean up my VesselState and push my VesselState today or tomorrow.