Skip to content

Add detailed comments for python example files (#1308)#1314

Merged
seanmcleod70 merged 3 commits intoJSBSim-Team:masterfrom
yuki-2000:Enrich-comments
Aug 4, 2025
Merged

Add detailed comments for python example files (#1308)#1314
seanmcleod70 merged 3 commits intoJSBSim-Team:masterfrom
yuki-2000:Enrich-comments

Conversation

@yuki-2000
Copy link
Contributor

Hello.

I added comments for beginners to the example files in examples/python, except for AoA vs CAS.ipynb because @agodemar just has improved it.

As a beginner of JSBSim, I added comments about what each variable I didn't understand meant, what the unit system was, jsbsim's unique settings, and the general flow of the whole process.
This time, I used the original comments as much as possible and did not change the code itself at all.

I apologize if @agodemar has already working on these 3 files.

Also, I found a few bugs in the code, so I will post them to issues later.

@codecov
Copy link

codecov bot commented Aug 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 24.81%. Comparing base (30a7261) to head (606f164).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1314   +/-   ##
=======================================
  Coverage   24.81%   24.81%           
=======================================
  Files         169      169           
  Lines       19631    19631           
=======================================
  Hits         4872     4872           
  Misses      14759    14759           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

"# Attempt to trim the aircraft.\n",
"try:\n",
" fdm['simulation/do_simple_trim'] = 1\n",
" # 1 means zero accelerations and constant attitude, usually for straight-and-level flight\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

Mostly correct, but 'constant attitude' isn't a requirement. The angular accelerations, p-dot, q-dot, r-dot need to be 0, but the angular velocities, p, q, r don't need to be 0. So you can trim with a specific roll rate, e.g. p=5deg/s.

See for more details, 1 == tFull:

typedef enum { tLongitudinal=0, tFull, tGround, tPullup,
tCustom, tTurn, tNone } TrimMode;
/*%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
CLASS DOCUMENTATION
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%*/
/** The trimming routine for JSBSim.
FGTrim finds the aircraft attitude and control settings needed to maintain
the steady state described by the FGInitialCondition object . It does this
iteratively by assigning a control to each state and adjusting that control
until the state is within a specified tolerance of zero. States include the
recti-linear accelerations udot, vdot, and wdot, the angular accelerations
qdot, pdot, and rdot, and the difference between heading and ground track.
Controls include the usual flight deck controls available to the pilot plus
angle of attack (alpha), sideslip angle(beta), flight path angle (gamma),
pitch attitude(theta), roll attitude(phi), and altitude above ground. The
last three are used for on-ground trimming. The state-control pairs used in
a given trim are completely user configurable and several pre-defined modes
are provided as well. They are:
- tLongitudinal: Trim wdot with alpha, udot with thrust, qdot with elevator
- tFull: tLongitudinal + vdot with phi, pdot with aileron, rdot with rudder
and heading minus ground track (hmgt) with beta
- tPullup: tLongitudinal but adjust alpha to achieve load factor input
with SetTargetNlf()
- tGround: wdot with altitude, qdot with theta, and pdot with phi

And

@property ic/p-rad_sec (read/write) Roll rate initial condition in radians/second
@property ic/q-rad_sec (read/write) Pitch rate initial condition in radians/second
@property ic/r-rad_sec (read/write) Yaw rate initial condition in radians/second

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the comment. I misunderstand the trim.
I understand that trim itself means stay with initial conditions, and trim mode means which acceleration component should be zero. Is this correct?

The comment should be # 1 means zero recti-linear and angular accelerations, usually for straight-and-level flight?
(while level have to be set by initial conditions.)

I am confused.
Someday, I would like to create examples of changeable property lists, trim mode lists, optimal usage situations, and so on.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think there is often a common misconception that trim implies that the angular velocities need to all be 0. Just like the translational velocities don't need to be 0, if they did have to be 0 then the only trim solutions would be stationary trim solutions on the ground, there is no requirement for the angular velocities to be 0, they can be 0, but they don't have to be, the only constraint for both the translational velocities and angular velocities is that they be constant, i.e. that their accelerations are 0.

Hence my comment that your original comment claiming that trim requires a constant attitude isn't accurate.

Take a look at the following paper for some more details on trim.

A General Solution to the Aircraft Trim Problem

image

Note that there are multiple ways to solve the trim problem, the one described in the paper I linked to is not currently available in JSBSim. The current trim algorithm in JSBSim is based on a different algorithm.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you very much. I read this paper, and found it very easy to understand. I realized that trim is actually very profound.
I understand that trim refers to determining a constant control values that brings the aircraft to a steady state where linear/angular accelerations are zero and linear/angular velocities are constant. And this condition is satisfied not only when flying straight, but also when performing steady turns or pull-up.
Also, I understand that level flight was achieved if the user set fdm['ic/h-sl-ft'].

The comment should be # 1 means straight flight by using all changeable control variables. right?

Copy link
Contributor

Choose a reason for hiding this comment

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

You can thank @agodemar regarding the paper since he's one of the authors and also a JSBSim developer 😉

Also, I understand that level flight was achieved if the user set fdm['ic/h-sl-ft'].

No, what will indicate that you want level flight is setting gamma $\gamma$, flight path angle to 0.

fdm['ic/gamma-deg'] = gamma

You need to set fdm[ic/h-sl-ft'] to some value even if you're requesting a trim that involves climbing or descending flight. So setting it doesn't request level (non changing altitude) trim. Gamma controls that, pretty sure it defaults to 0 if you don't explicitly set it.

1 - tFull - Longitudinal and lateral trim.

Copy link
Contributor

@agodemar agodemar Aug 4, 2025

Choose a reason for hiding this comment

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

@yuki-2000 yes, trimming a dynamical system is a subtle subject, especially when the system is a flying vehicle in atmosphere.
One example of subtlelty is this:
When you trim for a flight along a rectilinear trajectory with a nonzero flight path angle ($\gamma\ne0$), you want the true airspeed to be maintained constant with a constant regulation of aerodynamic control surfaces. This my be approximately true for a limited time interval. If you let the state propagate, you'll realize that the altitude is changing (because of nonzero $\gamma$), hence air density $\rho$ is changing, hence external forces and moments are changing.

So, rigourously, to keep a steady airspeed magnitude (and steady angle of attack $\alpha$ and a steady sideslip $\beta$) along a climbing or descending trajectory, one should continuously change flight control deflections and throttle setting. This is why sometimes you ear the term "instantaneous trim".
A similar reasoning is valid for a pull-up condition.

Copy link
Contributor

Choose a reason for hiding this comment

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

You run into this issue of the trim only being valid in the near term even for level trim cases, i.e. $\gamma = 0$. For example given fuel burn both the mass and the moments of inertia will be changing after the trim has been computed.

Now 'near term' is larger for this case than the climbing/descending case.

JSBSim offers a property to prevent the fuel burn affecting the mass and moments of inertia.

propulsion/fuel_freeze

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand that trim the plane just once doesn't mean it will stay the state forever. Thank you for your lecture.
So, does real commercial air plane trim itself at regular intervals repeatedly ?

Copy link
Contributor

Choose a reason for hiding this comment

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

In effect yes, either the pilot if they are hand-flying or the autopilot when it's engaged, which for commercial airliners is for a large percentage of the overall flight.

So for example the autopilot would be configured to maintain some altitude, e.g. 33,000ft at some speed, e.g. Mach 0.82 and it will then continuously modify the control surfaces and the engine throttles to maintain that.

Out of interest, what is your background and what are you using JSBSim for?

Copy link
Contributor Author

@yuki-2000 yuki-2000 Aug 4, 2025

Choose a reason for hiding this comment

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

Out of interest, what is your background and what are you using JSBSim for?

Well, I studied aerospace engineering in the university, so I could somehow keep up with your discussions. However, my research field in the university was structure. I wanted to learn the flight dynamics and control for a step up or make flight simulator using UE5 as a part of my hobby (I do not know whether I can do it by the way)
I just find JSBSim just a few month ago from UE5`s antoinette project website, and tried to build in my environment and find it very hard. Therefore, I learn JSBSim little by little.
I might dissapear if I lose interest in the simulator or my job gets busy, but I try to continue studying.

"\n",
"def thrust_vector_range_test(altitude, speed, flight_path_angle, title):\n",
" # altitude: altitude above sea level (ft)\n",
" # speed: mach number of speed (<1)\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

Detail that speed can be Mach number if it's < 1 as you have, but also mention that > 1 specific KCAS (Knots Calibrated Air Speed).

" # Track minimum thrust\n",
" min_thrust = 1000000\n",
" min_angle = 100\n",
" # Initialize the minimum thrust and thrust vectoring angles to a very large number.\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

Mention it's to track/record the minimum thrust and the angle at which the minimum occurs.

" thrust = fdm[\"propulsion/engine[0]/thrust-lbs\"]*2 # because there are two engines\n",
" thrusts.append(thrust)\n",
"\n",
" # update the minimum thrust and thrust vectoring angles.\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

Start sentence with capital letter, i.e. # Update the....

"altitude = 15000\n",
"min_gamma = -10\n",
"max_gamma = 10\n",
"min_speed = 120 # Set the minimum airspeed (kts).\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

Mention for the min and max speeds that it's CAS

" # Trim\n",
" # Trim the aircraft.\n",
" try:\n",
" # 1 means zero accelerations and constant attitude, usually for straight-and-level flight\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

See earlier comment about comment about 1 - tFull trim comment.

@seanmcleod70
Copy link
Contributor

Thanks @yuki-2000, overall looks good, have added a couple of minor comments.

@yuki-2000
Copy link
Contributor Author

I fix the comments based of above discussions.
Also the small bug #1315 is also fixed.

@yuki-2000
Copy link
Contributor Author

Oh, what happened to the test?
I only changed ipynb files, not c++ files.

@seanmcleod70
Copy link
Contributor

Yep, don't worry about that 1 particular failure. As you said you only modified .ipynb files, but by default any commit kicks off the full CI in terms of rebuilding everything and testing it etc.

If you know for sure your commits don't modify any files that require a build to be kicked off, e.g. editing documentation like the README or these .ipynb files then you can add a [skip ci] label to the commit description to prevent the rebuild etc.

@seanmcleod70 seanmcleod70 merged commit d0e6ef7 into JSBSim-Team:master Aug 4, 2025
24 of 25 checks passed
@seanmcleod70
Copy link
Contributor

Thanks @yuki-2000, I've just merged your PR in.

@yuki-2000 yuki-2000 deleted the Enrich-comments branch August 4, 2025 13:15
@yuki-2000
Copy link
Contributor Author

Thank you very much.

@dershow
Copy link

dershow commented Aug 4, 2025 via email

@bcoconni
Copy link
Member

bcoconni commented Aug 6, 2025

Yep, don't worry about that 1 particular failure. As you said you only modified .ipynb files, but by default any commit kicks off the full CI in terms of rebuilding everything and testing it etc.

If you know for sure your commits don't modify any files that require a build to be kicked off, e.g. editing documentation like the README or these .ipynb files then you can add a [skip ci] label to the commit description to prevent the rebuild etc.

This kind of inconvenience should be avoided by prepending the PR title with [skip ci] which should normally prevent the CI workflow from being run. PRs which do not modify the C++ code and/or the Python tests (located in the folder tests) are good candidates for that.

Also speaking about good practices, I am not fond of inserting the discussion/issue number in the PR title:

  1. The number will end up in the commit message which will already include the PR number thus making redundant information in the commit message, with no real added value in exchange (at least in my opinion).
  2. Due to the design of the GitHub platform, the issue number (# 1308) in the PR title can't be clicked and it does not create automatic cross links between the issue and the PR. So I would rather include the issue/discussion number in the PR description (i.e. first message) so that it can be clicked and it creates cross links.

Just some food for thoughts.

@bcoconni
Copy link
Member

bcoconni commented Aug 6, 2025

If you know for sure your commits don't modify any files that require a build to be kicked off, e.g. editing documentation like the README or these .ipynb files then you can add a [skip ci] label to the commit description to prevent the rebuild etc.

This kind of inconvenience should be avoided by prepending the PR title with [skip ci] which should normally prevent the CI workflow from being run. PRs which do not modify the C++ code and/or the Python tests (located in the folder tests) are good candidates for that.

Well, this is redundant information from myself 😄 I've read your message too fast @seanmcleod70

@seanmcleod70
Copy link
Contributor

Well, this is redundant information from myself 😄 I've read your message too fast @seanmcleod70

No problem, I've done that myself a couple of times with some of your comments 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants