Skip to content
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

Fixed inertia matrix error #502

Merged
merged 6 commits into from
Oct 17, 2021
Merged

Fixed inertia matrix error #502

merged 6 commits into from
Oct 17, 2021

Conversation

likangbei
Copy link
Contributor

Under the assumption that in the structural frame the:

  • X-axis is directed afterwards,
  • Y-axis is directed towards the right,
  • Z-axis is directed upwards,
    And then a 180 degree rotation is done about the Y axis so that the:
  • X-axis is directed forward,
  • Y-axis is directed towards the right,
  • Z-axis is directed downward.
    The original ixx,iyy,izz,ixy,ixz,iyz in structural frame.
    So, When transform the inertia products from the structural frame to the body frame, ixy and iyz should be reversed.

Under the assumption that in the structural frame the:
 - X-axis is directed afterwards,
 - Y-axis is directed towards the right,
 - Z-axis is directed upwards,
And then a 180 degree rotation is done about the Y axis so that the:
 - X-axis is directed forward,
 - Y-axis is directed towards the right,
 - Z-axis is directed downward.
The original ixx,iyy,izz,ixy,ixz,iyz in structural frame.
So, When transform the inertia products from the structural frame to the body frame, ixy and iyz should be reversed.
@seanmcleod
Copy link
Member

@likangbei have you taken a look at this issue - #125

@likangbei
Copy link
Contributor Author

@likangbei have you taken a look at this issue - #125

Ixz defined by DHC6 XML is wrong, it should be positive. It is recommended to consult the book《Airplane Flight Dynamics and Automatic Flight Controls part1》,It contains a large number of other aircraft ixz data。Of course, many flight dynamics books contain a large number of aircraft ixz data, many of which are actual values. If the ixz error of dhc6 is regarded as correct, it is not difficult to understand the error of inertia matrix. Is the ixz data source of dhc6 reliable? Or is it calculated using a non-standard structural coordinate system? As an aviation engineer, I think it is very likely to be wrong.

@bcoconni
Copy link
Member

bcoconni commented Oct 12, 2021

Following the discussion we had in #125, I'd also suggest that we take opportunity of this PR to allow supplying the inertia tensor in either the structural or the body frame.

@likangbei you could modify this PR to allow JSBSim:

  1. Reading the inertia components Ixy, Ixz and Iyz with the same sign convention than classical flight dynamics textbooks.
  2. Reading the inertia tensor in either the body or the structural frame.

This could be achieved by adding options to <mass_balance>:

  • <mass_balance frame="BODY|STRUCTURAL"> to supply the inertia tensor in the body or structural frame.
  • <mass_balance sign_convention="textbooks"> to read the non diagonal components of the inertia tensor
  • Any combination thereof.

Or any other option that you might think of.

@bcoconni
Copy link
Member

bcoconni commented Oct 12, 2021

Some further insight, as @aarondewindt mentioned in #440 that some sources assume Ixz to be:
$$I_{xz}=-\int xzdm$$
which is precisely the sign convention that JSBSim uses.

So the parameter naming might not be sign_convention="textbooks" but another wording to make the chosen sign convention understandable. Suggestions ?

@seanmcleod
Copy link
Member

@bcoconni while I was refreshing my memory in terms of products of inertia I came across the Wikipedia page - https://en.wikipedia.org/wiki/Moment_of_inertia#Inertia_tensor

Which in particular mentions both conventions.

There are some CAD and CAE applications such as SolidWorks, Unigraphics NX/Siemens NX and MSC Adams that use an alternate convention for the products of inertia. According to this convention, the minus sign is removed from the product of inertia formulas and instead inserted in the inertia matrix:

But in the end the sign is either in the definition of the product of inertia or in the inertia matrix.

@bcoconni
Copy link
Member

@seanmcleod thanks for refreshing my memory as well 👍. Since the topic is a recurring one, I'd guess this is a candidate for the FAQ ?

@aarondewindt
Copy link
Contributor

aarondewindt commented Oct 12, 2021

@bcoconni Yea, using the name textbook is probably not a good idea, I have the feeling I've come both across in textbooks.

I've seen people refer to JSBSim's current convention as negated, since the product of inertia is already negated. So I would suggest <mass_balance negated_inertia="true|false">? I'll have a look at my old notes tomorrow to see if I find some other more relevant names. It's not a reference frame thing, it really is only the sign convention.

In the meantime I think this page from Mathworks explains the issue quite nicely. JSBSim and Simscape use the same convention, so adding a similar (or same) explanation to the JSBSim's docs with a link in the FAQ would help people with the same issue.
https://www.mathworks.com/help/physmod/sm/ug/specify-custom-inertia.html

@likangbei
Copy link
Contributor Author

likangbei commented Oct 13, 2021

@bcoconni
At present, in the field of flight dynamics, the standard ixz definition is as follows
image
not

$$I_{xz}=-\int xzdm$$

Many people with basic knowledge of flight dynamics cannot realize this problem when using jsbsim. They often use jsbsim in professional research fields, which often require accurate simulation. Of course, if you mistake the sign of ixz, you naturally don't care much about such simulation errors for flight games such as FlightGear

Adding additional tags is not a good idea. The best way is to be compatible with the standard definitions in the field of flight dynamics. After all, jsbsim is a professional flight simulation software

In some current aircraft XML files, for example, ixz of B737 is defined according to the flight dynamics standard. Obviously, the author of XML does not realize that ixz in jsbsim is different from the standard definition

Adding options to <mass_balance> is Not a good idea . If it is not changed to the default standard definition, at least remind all users clearly

@aarondewindt
Copy link
Contributor

@likangbei Yea, I get what you're saying and the same happened to me a while back. However JSBSim has been under development for a long time, so changing convention will most likely cause more issues than solve anything.

Adding the documentation will at least give a explanation of the issue and the new option should draw the attention of those filling the xml to it.

@aarondewindt
Copy link
Contributor

aarondewindt commented Oct 13, 2021

@bcoconni another way to solve the issue is to allow users to fill in the inertia matrix directly simmilar to how tabels are filled in, there is no ambiguity on the sign convention there.

This could be the new recommended way and the current method can be depreciated and left there for backwards compatibility.

@seanmcleod
Copy link
Member

The best way is to be compatible with the standard definitions in the field of flight dynamics.

However if we did that without the use of additional tags etc. then we would break existing models, both models used by enthusiasts, some of whom may well not notice, but also models by professionals who went to the trouble to get the sign right etc. and who will now see different results when installing a new version of JSBSim.

I'd suggest going with @bcoconni's suggestion of two optional tags, i.e. to specify the sign convention and to specify the frame of reference, i.e. structural or body.

Plus adding some documentation, along the lines of a subset of the Mathworks page, adding some specific documentation with regards to the structural versus body frame.

Copy link
Contributor Author

@likangbei likangbei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am agree to your opinion.

@seanmcleod
Copy link
Member

suggest <mass_balance negated_inertia="true|false">

I suggest negated_crossproduct_inertia in case users think the negation also applies to the moments of inertia.

@likangbei
Copy link
Contributor Author

@seanmcleod @bcoconni @aarondewindt
In order to maintain the compatibility of existing models, add an option to judge whether the inertia product is negative.
FGMassBalance.cpp is modified as follows

  if (document->GetAttributeValue("negated_inertia") == string("false")) 
    return FGMatrix33( bixx,  bixy, -bixz,
                       bixy,  biyy,  biyz,
                      -bixz,  biyz,  bizz );
  else
    return FGMatrix33( bixx, -bixy,  bixz,
                      -bixy,  biyy, -biyz,
                       bixz, -biyz,  bizz );
  }

Adding options to <mass_balance>:
<mass_balance negated_inertia="false">

The default is negated_inertia="true". However, I still recommend explicit definitions so that the default can be changed in the future. Reduce the possibility of unconscious errors. In addition, it is suggested to distinguish the versions of XML models to reduce the burden of maintaining compatibility in the process of software upgrade.

@likangbei
Copy link
Contributor Author

likangbei commented Oct 14, 2021

I suggest negated_crossproduct_inertia in case users think the negation also applies to the moments of inertia.
negated_crossproduct_inertia It feels too long?
moments of inertia can't be negative, as we all know

@seanmcleod
Copy link
Member

Yes negated_crossproduct_inertia is more verbose, but it's not as if you'll need to type it more than once in the model file.

Take non-professional users for want of a better description, who may not even be aware of the two different conventions for the cross products of inertia, they glance at existing models when creating their own etc. and potentially think the negation refers to all the inertia values they need to supply, based on some JSBSim requirement etc.

So I'd rather err on the side of verbosity/clarity.

@likangbei
Copy link
Contributor Author

There is such code in the file:

  Ixx = mJ(1,1);
  Iyy = mJ(2,2);
  Izz = mJ(3,3);
  Ixy = -mJ(1,2);
  Ixz = -mJ(1,3);
  Iyz = -mJ(2,3);
// Calculate inertia matrix inverse (ref. Stevens and Lewis, "Flight Control &
// Simulation")

It is easy for developers to confuse the definition of ixz. Fortunately, it will not cause errors
In addition, the comments and instructions in the header file should be modified later

Copy link
Member

@bcoconni bcoconni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update @likangbei ! 👍
I have suggested a few changes for the documentation part of your PR. Comments and suggestions are welcome.

src/models/FGMassBalance.h Show resolved Hide resolved
@bcoconni
Copy link
Member

However, I still recommend explicit definitions so that the default can be changed in the future. Reduce the possibility of unconscious errors. In addition, it is suggested to distinguish the versions of XML models to reduce the burden of maintaining compatibility in the process of software upgrade.

I agree. I guess the JSBSim project should lead as an example meaning we should update all the models in the aircraft\ folder with <mass_balance negated_inertia="true"> ?

Is it easy for us to understand such notes? Are there any better suggestions and comments
@likangbei
Copy link
Contributor Author

I agree. I guess the JSBSim project should lead as an example meaning we should update all the models in the aircraft\ folder with <mass_balance negated_inertia="true"> ?

Yes, I've thought about it too. I think it's necessary to modify all aircraft configuration files. Many people will learn or modify based on the existing aircraft configuration file. Adding "negative_crossproduct_inertia" will help remind users to pay attention to the sign of ixz, which may be more effective than FAQ to reduce errors.

@bcoconni bcoconni merged commit 7a9514c into JSBSim-Team:master Oct 17, 2021
@bcoconni
Copy link
Member

PR merged to the codebase. Thanks @likangbei !

For information the build fails for MSVC but the cause of this failure is unrelated to @likangbei's PR.

@bcoconni
Copy link
Member

Yes, I've thought about it too. I think it's necessary to modify all aircraft configuration files. Many people will learn or modify based on the existing aircraft configuration file. Adding "negative_crossproduct_inertia" will help remind users to pay attention to the sign of ixz, which may be more effective than FAQ to reduce errors.

Indeed and this change can easily be done via a script.

bcoconni pushed a commit to bcoconni/jsbsim that referenced this pull request Nov 6, 2021
…am#502)

By setting this option to false, the cross product inertia can be specified with the sign convention used in classical flight dynamics textbooks such as Steven & Lewis' or Etkin's.
bcoconni added a commit to bcoconni/jsbsim that referenced this pull request Nov 6, 2021
Folowing the PR JSBSim-Team#502, JSBSim now allows to specify how the sign of cross product inertia is interpreted. In order to educate users to this new parameter, all the aircraft models with cross product inertia are updated with the new parameter `negated_crossproduct_inertia` set to `true` (which is the default).
@bcoconni
Copy link
Member

bcoconni commented Nov 6, 2021

I agree. I guess the JSBSim project should lead as an example meaning we should update all the models in the aircraft\ folder with <mass_balance negated_inertia="true"> ?

Yes, I've thought about it too. I think it's necessary to modify all aircraft configuration files. Many people will learn or modify based on the existing aircraft configuration file. Adding "negative_crossproduct_inertia" will help remind users to pay attention to the sign of ixz, which may be more effective than FAQ to reduce errors.

This is now done in the commit 9a1b7db. I also pushed to the repo the Python script admin/XML_mass_update.py that I used to automatically update the aircraft files. It might be useful someday.

bcoconni added a commit to bcoconni/jsbsim that referenced this pull request Nov 6, 2021
Folowing the PR JSBSim-Team#502, JSBSim now allows to specify how the sign of cross product inertia is interpreted. In order to educate users to this new parameter, all the aircraft models with cross product inertia are updated with the new parameter `negated_crossproduct_inertia` set to `true` (which is the default).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants