Re: [Flightgear-devel] heli simulation update ready for cvs?

[Maik J. <mj...@gm...>]

Hi Andy,
Andy Ross wrote:
> You have misinterpreted the code, I think.  The "stallLift" value is
> only one contribution to the transverse force from the surface.
> Regular "drag" forces still apply, and I assure you they work.  If
> they did not, YASim aircraft would not be stable when placed in
> zero-velocity situations like a hammerhead stall or similar situation
> where AoA goes through 90.
>
>   
I will check this out
>> The fuselage is simulated as a open tube and in the bo the resulting
>> drag was to small I think. Therefore I added a vertical stab to
>> close the tube at the beginning. This vertical slab I realized by an
>> stab with 90 deg. incidence. 
> The performance cost here is rather high, this code gets hit at 400 Hz
> for every surface in the aircraft (and there can be hundreds on a big
> plane).  The small angle trick was being used for a reason, basically.
>
>   
ok, I understand
> * Suggestion A: an "effectiveness" parameter for fuselage tags that
>     works like it does for wings, so you can tune it per-aircraft.
>   Suggestion B: a new "spheroid" type that creates surfaces
>     distributed in 3D around a center point instead of the 1D
>     distribution you get from a fuselage tag.  This would probably
>     still need an effectiveness tunable, but will be much easier for
>     authors of helicopters and airships to use than vstab objects.
>
>   
Both sound very reasonable
>> Andy Ross wrote:
>>     
>>> The boundary changes ...
>>>       
> Ah, OK.  Yes, this was indeed a bug; I'm kinda shocked that we never
> noticed this before, it's been there since the code was written.  And
> unfortunately it affects *all* wings with segments that like between a
> control position and an edge.  Which means that we're going to need to
> retest the solution results for basically every YASim aircraft once
> this goes in.  Ick. :(
>   
I have calculated the solver results for nearly all YASim aircrafts 
(with and without patch) to find sinful solver values for the bo (I 
hoped to find some general correlation between aircraft parameters (like 
wingspan, weight) and the solver results, but I didn't). For some planes 
the result is identical, for most planes the values differ some percent, 
and for three planes the values differ more than 10%. What I do not have 
in the tabular, is the vstab. Some planes will have significant larger 
vstabs with the patch. But the tabular gives a hint, which planes to 
check first. (tabular is enclosed).
> As always, it's nicer to hear about bugs as bug reports, and not as
> anonymous changes in a 120k patch file. :)
Oh, I posted a mail "[Flightgear-devel] state of heli simulation, bug in 
Yasim" in mid of July. Next time (but I hope there is no need) I will 
write a single mail with a bug report.
>  Can you (1) split this out
> into a separate patch as above, (2) add a comment explaining the two
> new entries in the table, and (3) write "10" instead of "8+2" (or
> BOUND_COUNT or sizeof(bounds)/sizeof(bounds[0]), etc...).
>   
(1): yes
(2): yes
(3): yes
>> Yes, I will change this. The lines [...]  will be replaced by one
>> function call.
>>     
>
> OK.  But why must this be in Model.cpp?  
no, the function will be outside this file. In a second step all this 
will go to a new class, probably RotorGear, which will be the interface 
to one yasim engine and all the rotors
> Leaving a comment in place above your code that you disagree with
> seems kinda silly; I don't care if I wrote it or not.  Pull it out and
> replace it with the above explanation of why this code is OK inside of
> initIteration() and does not need to be in calcForces().
>
>   
I will do this.
> A more general suggestion: isn't this a design bug with your choice of
> coordinates?  The body is an accelerated reference frame.  Wouldn't it
> be better to store the rotor orientation as a matrix in the global
> frame instead, the same way aircraft orientations are stored?  Not
> only would you be able to skip this step, you would be immune to
> physics bugs caused by rapid rotations of the helicopter body.
>   
I don't think so. The incidence of each rotor blade is a delta to the 
fuselage orientation. The force/torque from rotor to fuselage depends 
very strongly to the difference between the two orientations. Therefore 
I think the rotor calculation is easier to understand with the actual 
ansatz.
>   
>> For debugging the rotor code I had switched the floating point
>> exceptions on. But I got floating point exceptions exactly in the
>> modified functions directly after starting the program, probably due
>> to uninitialized variables somewhere else.
>>     
>
> OK, then we need to fix *those* bugs (which, as above, will never
> happen if you don't report them!).  Your patch only hides them, and it
> does so incorrectly.  If you don't want to track the original issue
> down, then better fixes would be:
>
> A. A zero _totalMass (which should *never* happen, and really should
>    be a fatal error if you were seeing it) can be approximated as a
>    suitably small number as long as you know the units
>    involved(e.g. 1.0e-06 kg will work fine without hitting any
>    truncation issues in single precision math).  Setting it to 1.0 is
>    just weird.
>
> B. A identically zero _spin vector (which, OK, is actually something
>    that might happen in principle) should be replaced with a nominal
>    unit vector like [0,0,1] or the like.  Your code actually isn't
>    technically incorrect (the resulting zero-length vector only gets
>    used in a dot product, which is safe) but is definitely confusing.
>   
I agree to you. As I switched on floating point exceptions for 
flightgear, I got so much of them, that I was not able to debug the FDM. 
So I assumed, that they are generally accepted in flightgear.
> This issue is a pet peeve of mine.  As a general rule, floating point
> code should never be testing a number for equality.  Doing so leads to
> just this kind of discontinuity: the behavior of the simulator is very
> different at one value than at one infinitesimally close.
>
>   
I agree to you. I got the exception with invalid parameters at startup 
of the flightsim. Therefore I thought , any valid result will do it 
better... But in generally you are 100% right.
>>> There are several spots where you have commented code out instead
>>> of removing it
>>>       
>> I hope I only did this where I modified your code to illustrate the
>> changes.
>>     
>
> This is another pet peeve: the right way to illustrate changes is to
> use a patch file and accompanying change comments (you gave me the
> former, but not the latter).  Commenting stuff out just guarantees
> that you will confuse someone later on ("what happens if I uncomment
> this?").  Just don't do it, code can only be maintained when it is
> *code*, not comments.
>   
ok, I will change my way of illustration to yours.

> Leaving a comment in place above your code that you disagree with
> seems kinda silly; I don't care if I wrote it or not.  Pull it out and
> replace
Than I should remove the 
// ACK: the dreaded ambiguous string boolean.  Remind me to shoot Maik
// when I have a chance. :).  
because I disagree to this to pure egoistic reason.
I have added a comment to README.yasim, to not use "true" and "false".
(Did really I added the "ambiguous string boolean? I thought, I did not, but I am getting older and older and to store on new bit in my brain I have to delete some bytes in advance.)


> Andy
>   
Maik