Re: [Flightgear-devel] heli simulation update ready for cvs?
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
Re: [Flightgear-devel] heli simulation update ready for cvs?
Maik Justus wrote: > my impression was, that a stab is not producing forces perpendicular to > its surface, when the wind is blowing perpendicular to its surface 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. > 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. There should be many other ways to do > this without a vertical stab, but this way was self-evident for me. OK, I understand. But this is a hack. Your problem has nothing to do with surface incidence: your problem is that the fuselage drag is too low. Since the solver isn't run for helicopters, that isn't surprising. Why not investigate solutions that address the actual problem* and don't involve putting trig operations into the inner loop of the simulator? If you *really* need this feature and don't want to use one of the IMHO more general features above, can you provide it to me as a separate patch with change comments? It really isn't related to helicopters. 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. * 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. > Andy Ross wrote: > > The boundary changes in Wing.cpp don't make any sense to me. > > Therefore for an wing without flap, spoiler and slat [...] no surface > element is generated. 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. :( As always, it's nicer to hear about bugs as bug reports, and not as anonymous changes in a 120k patch file. :) 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...). > Yes, I will change this. The lines [...] will be replaced by one > function call. OK. But why must this be in Model.cpp? That class is a top-level wrapper that contains a bunch of sub-objects to do the real simulation; it doesn't have any aircraft-specific logic. Shouldn't this be part of a Rotor? If not, maybe you need another abstraction? (RotorShaft, maybe, by analogy to PropEngine?) The immediate complaint was that this was a lot of very special-purpose code inside my a small, general function. But the proximate issue remains: this seems like a lot of special-purpose code without an obvious "home" in the design. Abstraction is nice: it lets your changes go into files that you "own" (like Rotor*.?pp) without being torn apart by viscious folk like me. :) > Of course I didn't want to change your comment. Yes, there is some > integration done [...] the compensation of the rotation of the > fuselage. The rotor does not follow the rotation of the > fuselage. Therefore its rotation must be subtracted from the > orientation of the rotor. I think, one could call this "unstiff > problems". 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(). 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. > 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 u
Re: [Flightgear-devel] heli simulation update ready for cvs?
Hi, Maik Justus wrote: > The outer definition of the wing in Yasim is given by the innermost and > outermost bound[]. But the bound[] are only defined at flapx start, > flapx end, spoiler... and slat... .Therefore for an wing without flap, > spoiler and slat the bound[] array is empty and no surface element is > generated. E. g. for the bo105 hstab (without flap, spoiler and slat) > this stab is not simulated in YASim. E. g. for the j3, with only one > control subelement at the wing ( drag="1.4"/>), there is no surface generated for the outer 15% of the > wing (the same for the inner 31%). > I have to correct this a little bit. For the J3 a surface is generated for the inner 31% due to the fact, that all control subelements, which are not defined, yield to a bound[x]=0. The innermost part is only missing for wings with 2 flaps, spoiler and slat. Maik - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys -- and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV ___ Flightgear-devel mailing list Flightgear-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/flightgear-devel
Re: [Flightgear-devel] heli simulation update ready for cvs?
Hi Andy, thank you for reviewing the patch. Andy Ross wrote: > Maik Justus wrote: > >> for my part the update of the yasim heli simulation is ready for >> cvs. Andy: therefore I want to ask you for a review of the patch >> and your agreement to add this to cvs or a list of necessary >> modifications or (hopefully not) a clear no. >> > > OK, here's a quick review of the stuff I don't like. There are no > comments here on the Rotor* files. Those are 100% yours and really > not my business to complain about. :) > > Explain why the changes to Surface.cpp are needed? The comments about > stall lift are incorrect -- YASim does not attenuate lift at stall to > zero. my impression was, that a stab is not producing forces perpendicular to its surface, when the wind is blowing perpendicular to its surface, as it is for the hstab for a helicopter in hover (valid only for helictopters with the hstab beneath the rotor). I think that this is to the fact, that "stallMul" becomes one for this case and therefore "stallLift = (stallMul - 1) * _cz * out[2]" becomes zero. This conditions will not occur in a plane and therefore this had no visible impact to planes. > And why do you need incidence values to be non-small angles? > If that's the requirement, then it would be better to have an > implementation that can rotate the surface orientation instead. Doing > it here loks like a hack. Do you actually need these changes for the > helicopter code? Why? > 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. There should be many other ways to do this without a vertical stab, but this way was self-evident for me. > The boundary changes in Wing.cpp don't make any sense to me. You are > adding two entries, but don't map them to anything. How is this > change not a complete (and hard to understand) no-op? > > The outer definition of the wing in Yasim is given by the innermost and outermost bound[]. But the bound[] are only defined at flapx start, flapx end, spoiler... and slat... .Therefore for an wing without flap, spoiler and slat the bound[] array is empty and no surface element is generated. E. g. for the bo105 hstab (without flap, spoiler and slat) this stab is not simulated in YASim. E. g. for the j3, with only one control subelement at the wing (), there is no surface generated for the outer 15% of the wing (the same for the inner 31%). > The changes to Model::calcForces() are just not acceptable. This > routine should be a short, top-level wrapper for the force calculation > (i.e. for each surface, set up parameters and calculate forces, then > do the same for each engine, etc...). You have dumped a *TON* of > quite clearly helicopter-specific logic right into the middle of this > function. (Amusingly, you also broke my comment that reads "end of > engine stuff" -- it now comes something like 100+ lines after the end > of the engines). Can't this go into one of the Rotor* files? At the > very least, get it out of calcForces(). > Yes, I will change this. The lines between the two comments // check, Also, I notice that I had a nit-picky comment at the top of > Model::initRotorIteration(). I know you read this comment, because > you modified the function. But you don't seem to have addressed the > concern. Either remove the comment if it is incorrect, explain to me > why it's not a problem, or fix the code. :) > Of course I didn't want to change your comment. Yes, there is some integration done in the "void Rotorpart::inititeration(float dt,float *rot)" called from this function. One is the integration of the "rotor orientation" by omega*dt for the 3D-visualization of the heli only. The other is the compensation of the rotation of the fuselage. The rotor does not follow the rotation of the fuselage. Therefore its rotation must be subtracted from the orientation of the rotor. I think, one could call this "unstiff problems". > The changes to RigidBody.cpp are just plain wrong. You appear to be > trying to handle a divide-by-zero condition by testing the > denominator. Which is sort of OK, if clumsy (AFAICT, neither of these > situations can occur in practice -- please explain why you needed to > fix these; I suspect you had some other bug). Unfotrunately, your > fallback cases treat the infinity that gets produced as being EQUAL TO > ONE! > 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. Maybe the exceptions were based on a bug in the heli simulation (due to the not invoked solver in heli simulation. I think I remember, that "setupWeights(true)" was missing there). I will set breakpoint
Re: [Flightgear-devel] heli simulation update ready for cvs?
Maik Justus wrote: > for my part the update of the yasim heli simulation is ready for > cvs. Andy: therefore I want to ask you for a review of the patch > and your agreement to add this to cvs or a list of necessary > modifications or (hopefully not) a clear no. OK, here's a quick review of the stuff I don't like. There are no comments here on the Rotor* files. Those are 100% yours and really not my business to complain about. :) Explain why the changes to Surface.cpp are needed? The comments about stall lift are incorrect -- YASim does not attenuate lift at stall to zero. And why do you need incidence values to be non-small angles? If that's the requirement, then it would be better to have an implementation that can rotate the surface orientation instead. Doing it here loks like a hack. Do you actually need these changes for the helicopter code? Why? The boundary changes in Wing.cpp don't make any sense to me. You are adding two entries, but don't map them to anything. How is this change not a complete (and hard to understand) no-op? The changes to Model::calcForces() are just not acceptable. This routine should be a short, top-level wrapper for the force calculation (i.e. for each surface, set up parameters and calculate forces, then do the same for each engine, etc...). You have dumped a *TON* of quite clearly helicopter-specific logic right into the middle of this function. (Amusingly, you also broke my comment that reads "end of engine stuff" -- it now comes something like 100+ lines after the end of the engines). Can't this go into one of the Rotor* files? At the very least, get it out of calcForces(). Also, I notice that I had a nit-picky comment at the top of Model::initRotorIteration(). I know you read this comment, because you modified the function. But you don't seem to have addressed the concern. Either remove the comment if it is incorrect, explain to me why it's not a problem, or fix the code. :) The changes to RigidBody.cpp are just plain wrong. You appear to be trying to handle a divide-by-zero condition by testing the denominator. Which is sort of OK, if clumsy (AFAICT, neither of these situations can occur in practice -- please explain why you needed to fix these; I suspect you had some other bug). Unfotrunately, your fallback cases treat the infinity that gets produced as being EQUAL TO ONE! And, just in general, I wouldn't mind a little code cleanup. There are several spots where you have commented code out instead of removing it; I see a few places where you have more than one blank line in a row; your brace/whitespace/naming conventions don't always match the surrounding code, etc... I'm not a big zealot about this stuff, though (except for the commented-out-code issue -- that drives me nuts). Andy - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys -- and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV ___ Flightgear-devel mailing list Flightgear-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/flightgear-devel
Re: [Flightgear-devel] heli simulation update ready for cvs?
Hi, there was a bug in the diff file (I have deleted some debug code directly in the diff file...). Please use this file instead of the diff file enclosed in the archive. Maik Maik Justus schrieb: Hello, for my part the update of the yasim heli simulation is ready for cvs. Andy: therefore I want to ask you for a review of the patch and your agreement to add this to cvs or a list of necessary modifications or (hopefully not) a clear no. The heli add on is not fully finished now, but the missings will have only very small impact to the flight dynamic. The only upcoming modification with impact to the xml configuration files of the helicopters will be the connection to the yasim engines. Probably I will do this by a rotorgear tag, which will be a modified propeller tag acting as an interface between one engine and all the rotors. But I need further modifications, because now there is no governor for the engines. For constant rpm propeller the incidence is varied to get the constant rpm, for a rotor the power must be modified to get constant rpm. And simulation of multi engine helicopters should be done by more than one engine... I have decided to include the patch directly to this mail due to the quite small size. I have to say thank you to Melchior for his assistance testing and improving this patch. Maik YASim.diff.gz Description: application/gzip - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys -- and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV___ Flightgear-devel mailing list Flightgear-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/flightgear-devel
[Flightgear-devel] heli simulation update ready for cvs?
Hello, for my part the update of the yasim heli simulation is ready for cvs. Andy: therefore I want to ask you for a review of the patch and your agreement to add this to cvs or a list of necessary modifications or (hopefully not) a clear no. The heli add on is not fully finished now, but the missings will have only very small impact to the flight dynamic. The only upcoming modification with impact to the xml configuration files of the helicopters will be the connection to the yasim engines. Probably I will do this by a rotorgear tag, which will be a modified propeller tag acting as an interface between one engine and all the rotors. But I need further modifications, because now there is no governor for the engines. For constant rpm propeller the incidence is varied to get the constant rpm, for a rotor the power must be modified to get constant rpm. And simulation of multi engine helicopters should be done by more than one engine... I have decided to include the patch directly to this mail due to the quite small size. I have to say thank you to Melchior for his assistance testing and improving this patch. Maik heli060731.tar.gz Description: application/gzip - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys -- and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV___ Flightgear-devel mailing list Flightgear-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/flightgear-devel
