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

Adding capability to aim external forces in inertial frame; added exa… #962

Merged
merged 5 commits into from
Sep 24, 2023

Conversation

jonsberndt
Copy link
Contributor

Added support for simple orbital phasing. This included adding a new frame in FGForce (tInertialBody). I added a new IC file to model an ISS orbit. I added a new script to run the example.

@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

Merging #962 (0e5f579) into master (c9b12c4) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #962      +/-   ##
==========================================
- Coverage   24.90%   24.89%   -0.01%     
==========================================
  Files         168      168              
  Lines       18890    18897       +7     
==========================================
  Hits         4704     4704              
- Misses      14186    14193       +7     
Files Changed Coverage Δ
src/models/FGExternalForce.cpp 0.00% <0.00%> (ø)
src/models/FGExternalForce.h 0.00% <ø> (ø)
src/models/propulsion/FGForce.cpp 0.00% <0.00%> (ø)
src/models/propulsion/FGForce.h 0.00% <ø> (ø)

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.

It's good to see back on board @jonsberndt 😄

Regarding your PR, I have made a couple of comments:

  1. Since you increased the ball empty weight, you should also increase the parachute area (line ~110 in ball.xml) by the same amount so that ball_chute.xml behaves the same way than before.
  2. I understand that the <direction> element of external forces is not used in your example. For the sake of clarity, I'd suggest removing it from the XML file, maybe with a comment saying that it has been omitted on purpose.

aircraft/ball/ball.xml Outdated Show resolved Hide resolved
aircraft/ball/ball.xml Show resolved Hide resolved
@seanmcleod
Copy link
Member

Simply commit the changes to your current local branch, and then push your local branch. They will then automatically show up here on the pull request with the additional commits you've made.

@jonsberndt
Copy link
Contributor Author

jonsberndt commented Sep 22, 2023

I've added the changes (I think) that were suggested above, and added additional orbital maneuvers. Note that these are for demonstrative purposes only and don't represent any actual vehicle.

With that said ... :-)

I also created a script in Octave that post-processes the data for plotting/verification and makes a plot. The plot assumes that:

A JSBSim run is first made with the "thrust" set to zero in the script, ball_orbit_phase.xml. That would be implemented by setting the "value" to zero in lines that look like this in each of the three "burn" events currently in the script:

      <set name="propulsion/rocket_thrust" value="2000" action="FG_RAMP" tc="1.0"/>

Above, the thrust is already set to 2000 (lbs).

Then, rename the BallOut.csv file that results from this run to BallOut_noburn.csv.

Following that, run the script again WITH the thrust values that are currently there in order to change the orbit. After that run is complete, rename the BallOut.csv file to BallOut_burn.csv, and you are ready for the octave script. I haven't added this to the repo - I'm not sure if that's appropriate and not sure where I would put it, so just going to share the octave script here:

% Read the burn and no-burn data files
% The "burn" trajectory represents a capsule that begins moving away from the space station.
% The "no-burn" trajectory represents an unperturned space station trajectory.

burn = csvread('BallOut_burn.csv');
noburn = csvread('BallOut_noburn.csv');

% Move the vector difference in inertial position into new variables

vecx = burn(:,121) - noburn(:,121);
vecy = burn(:,122) - noburn(:,122);
vecz = burn(:,123) - noburn(:,123);
vecdist=[vecx vecy vecz];

% Calculate the magnitudes of the vector distances between the non-perturbed 
% and perturbed trajectories

vecdist_mag = sqrt(vecdist(:,1).^2 + vecdist(:,2).^2 + vecdist(:,3).^2);
radialdist_mag = burn(:,132) - noburn(:,132);

% Need to calculate the horizontal distance properly which requires a look at
% the relative longitudinal differenes

long_diff = noburn(:,120) - burn(:,120);
for idx=1:length(long_diff)
  if abs(long_diff(idx)) > 180
    if (long_diff(idx) > 0) 
      long_diff(idx) = long_diff(idx) - 360;
    else
      long_diff(idx) = long_diff(idx) + 360;
    end
  end
end

% The sign of the difference indicates which vehicle is ahead and which trails in orbit

mysign = sign(long_diff);

trackdist_mag = mysign.*sqrt(vecdist_mag.*vecdist_mag - radialdist_mag.*radialdist_mag);

% Plot vertical (radial) separation versus horizontal tracking distance as vehicles separate

plot(trackdist_mag/5280, radialdist_mag/5280);

This pair of runs shows a spacecraft undocking from a space station from a forward facing port, such that in undocking the speed increases and subsequently orbital altitude does as well. As altitude increases, velocity decreases and the capsule flies above and behind the station. At the new apogee another burn is made (slowing the capsule) and it drops down behind the station and in a lower orbit. Half an orbit later, anothern bur is made to circularize the orbit.

@jonsberndt
Copy link
Contributor Author

jonsberndt commented Sep 22, 2023

It also looks like some of the changes I made have introduced some failures. Not sure what happened ... works on my end.

Edit: After further review it looks like the external reactions test is failing. Is this because the script used for the test now produces different results than it previously did - ostensibly because I changed the mass of the ball?

@seanmcleod
Copy link
Member

works on my end

Yep, was going to ask if you had run https://github.com/JSBSim-Team/jsbsim/blob/master/tests/TestExternalReactions.py

So when I run it locally using JSBSim master it passes with the following output.

(base) C:\source\jsbsim\tests>python .\TestExternalReactions.py
test_body_frame (__main__.TestExternalReactions) ... GEAR_CONTACT: 0 seconds: NOSE_LG 1
GEAR_CONTACT: 0 seconds: LEFT_MLG 1
GEAR_CONTACT: 0 seconds: RIGHT_MLG 1
ok
test_moment (__main__.TestExternalReactions) ...
  The aerodynamic moment axis system has been set by default to the bodyXYZ system.

Reef 1 (Event 0) executed at time: 30.3821
    simulation/sim-time-sec = 30.3821


Reef 2 (Event 1) executed at time: 31.8654
    simulation/sim-time-sec = 31.8654


Reef Final (Event 2) executed at time: 34.5903
    simulation/sim-time-sec = 34.5903

GEAR_CONTACT: 37.0985 seconds: CONTACT 1

Terminate (Event 3) executed at time: 37.1068
    simulation/sim-time-sec = 37.1068

*CRASH DETECTED* 37.1068 seconds: CONTACTok
test_wind_frame (__main__.TestExternalReactions) ...
  The aerodynamic moment axis system has been set by default to the bodyXYZ system.

Reef 1 (Event 0) executed at time: 30.3821
    simulation/sim-time-sec = 30.3821


Reef 2 (Event 1) executed at time: 31.8654
    simulation/sim-time-sec = 31.8654


Reef Final (Event 2) executed at time: 34.5903
    simulation/sim-time-sec = 34.5903

GEAR_CONTACT: 37.0985 seconds: CONTACT 1

Terminate (Event 3) executed at time: 37.1068
    simulation/sim-time-sec = 37.1068

ok

----------------------------------------------------------------------
Ran 3 tests in 1.573s

OK
(base) C:\source\jsbsim\tests>

If you look at the error log during the automated build test I noticed this difference.

No direction element specified in force object. Default is (0,0,0).
Start 30: TestExternalReactions
 6/71 Test #30: TestExternalReactions ............***Failed    1.62 sec
test_body_frame (__main__.TestExternalReactions) ... GEAR_CONTACT: 0 seconds: NOSE_LG 1
GEAR_CONTACT: 0 seconds: LEFT_MLG 1
GEAR_CONTACT: 0 seconds: RIGHT_MLG 1
ok
test_moment (__main__.TestExternalReactions) ... 
In file ./aircraft/ball/ball.xml: line 124
No direction element specified in force object. Default is (0,0,0).

  The aerodynamic moment axis system has been set by default to the bodyXYZ system.

Reef 1 (Event 0) executed at time: 30.3821
    simulation/sim-time-sec = 30.3821

FAIL
test_wind_frame (__main__.TestExternalReactions) ... 
In file ./../../../aircraft/ball/ball.xml: line 134
No direction element specified in force object. Default is (0,0,0).

  The aerodynamic moment axis system has been set by default to the bodyXYZ system.

Reef 1 (Event 0) executed at time: 30.3821
    simulation/sim-time-sec = 30.3821

FAIL

======================================================================
FAIL: test_moment (__main__.TestExternalReactions)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/jsbsim/jsbsim/tests/TestExternalReactions.py", line 236, in test_moment
    self.assertAlmostEqual(fdm['forces/fbx-external-lbs'], f[0, 0])
AssertionError: 7688.1197741397 != 15.376239455482793 within 7 places (7672.743488 difference)

@jonsberndt
Copy link
Contributor Author

I'm not exactly sure what the specific cause of the failure is, after looking at this. Will have to revisit it later. Do you have any suggestions?

@seanmcleod
Copy link
Member

Okay I think I've spotted the issue.

So note the parachute area of 20 which is used to calculate the force.

<force name="parachute" frame="WIND">
<function>
<product>
<property>aero/qbar-psf</property>
<property>fcs/parachute_reef_pos_norm</property>
<value> 1.0 </value> <!-- Full drag coefficient -->
<value> 20.0 </value> <!-- Full parachute area -->
</product>
</function>

And then note that the force is calculated in TestExternalReactions.py on line 57, in which the parachute area of 20 is hard-coded in the test script.

mag = fdm['aero/qbar-psf'] * fdm['fcs/parachute_reef_pos_norm']*20.0
f = Tw2b * np.mat([-1.0, 0.0, 0.0]).T * mag
self.assertAlmostEqual(fdm['forces/fbx-external-lbs'], f[0, 0])

Now in your commit, after @bcoconni's suggestion, you've increased the area of the parachute in ball.xml but the test script is still using the old parachute area when calculating and comparing the force during the test run.

@jonsberndt
Copy link
Contributor Author

Well, I thought I fixed it, but it appears there is still a failure and I can't see what's wrong - not familiar with this python testing setup.

@jonsberndt
Copy link
Contributor Author

Looking at the test that I believe failed, test_moment, that test uses the ball_chute script - not the new ball_orbit_phase.xml script.

The parachute is reefed in three stages as a percentage of its total surface area. This would result in stages where the drag force and moment are entirely different than what was there previously. Since this is done in the ball.xml vehicle file, not sure we can maintain backwards compatibility with previous results. If this test involves comparing with the previous results, it will fail and I don't think there's any way to fix that. If it is desirable to maintain backwards compatibility with the previous test I could dispense with this use of the ball example and make a new one, such as capsule or something.

@seanmcleod
Copy link
Member

seanmcleod commented Sep 23, 2023

not sure we can maintain backwards compatibility with previous results. If this test involves comparing with the previous results

Yep, but this particular test doesn't do a comparison with a previously stored set of results.

The issue is that there are two places in this test script where the parachute area is used in the python calculation and you only updated the one location 😉

mag = fdm['aero/qbar-psf'] * fdm['fcs/parachute_reef_pos_norm']*20.0

And

mag = fdm['aero/qbar-psf'] * fdm['fcs/parachute_reef_pos_norm']*20.0

Ideally we wouldn't have this hard-coded parachute area in the python code, rather it would be specified as a property which the python code then could query when needing to use it in it's force calculation.

@bcoconni
Copy link
Member

Ideally we wouldn't have this hard-coded parachute area in the python code, rather it would be specified as a property which the python code then could query when needing to use it in it's force calculation.

I'm in total agreement with this statement so I took the liberty to modify the code of this PR and replace the hardcoded values of the parachute area in TestExternalReactions.py by a value that is extracted from the XML definition file of ball.

@bcoconni
Copy link
Member

The tests have just succeeded so let me know if you are happy with the changes I made to TestExternalReactions.py (the rest of the PR code is pristine as per @jonsberndt submission).

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.

So as it looks good to everyone, I'll merge the PR.
Thanks ! 👍

@bcoconni bcoconni merged commit ae4652a into JSBSim-Team:master Sep 24, 2023
29 checks passed
bcoconni added a commit to bcoconni/jsbsim that referenced this pull request Sep 24, 2023
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.

None yet

3 participants