Lookup tables in more than 3 dimensions

[agodemar]

Pull request as per discussion #1425 (reply in thread)

[bcoconni]

Thanks for the PR @agodemar. I think you need to clean it up a bit:

• Please remove the files matlab-sfunction-ubuntu.yml and matlab-sfunction.yml from the folder .github/workflows
• Please remove the changes to cpp-python-build.yml in the folder .github/workflows.
• Please do not add to .gitignore any *.yml file from the folder .github/workflows. Otherwise any future change to cpp-python-build.yml will be ignored by git.
  • Said otherwise, the following lines should be removed from .gitignore:

.github/workflows/matlab-sfunction-ubuntu.yml
.github/workflows/matlab-sfunction.yml
.github/workflows/cpp-python-build.yml

• Please remove the file BRANCH_NOTES.md from the branch.
• Please remove the changes to README.md: you have added a link to your "GitHub Codespace", I do not see much point in including it in the official repo.

I have not yet reviewed the rest of your changes, the suggestions above are only meant to focus the PR to the changes that are directly related to the N dimensional interpolation.

[codecov]

Codecov Report

❌ Patch coverage is 83.82979% with 38 lines in your changes missing coverage. Please review.
✅ Project coverage is 25.18%. Comparing base (3896d4a) to head (01d43d6).

Files with missing lines	Patch %	Lines
src/math/FGTable.cpp	83.33%	38 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1443      +/-   ##
==========================================
+ Coverage   25.11%   25.18%   +0.06%     
==========================================
  Files         169      169              
  Lines       18721    18783      +62     
==========================================
+ Hits         4702     4730      +28     
- Misses      14019    14053      +34     

☔ 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.

[agodemar]

@bcoconni thank you for your suggestions. I cleand up the PR as you requested

[bcoconni]

@agodemar I have only reviewed the changes you made since the comments I made in #1425 (comment).

So I made a few comments that, hopefully should make the code slightly easier to read (at least for me).

[bcoconni]

I have pushed a couple of commits to this PR to restore the CI workflow (i.e. the file .github/workflows/cpp-python-build.xml and fix a crash in CheckAircrafts.py.

Also I have renamed the PR to the title of the discussion #1425 that you have initiated: Lookup tables in more than 3 dimensions. I think this is clearer than N dimensional interpolation (2) 😃

Finally, for the record, this PR will fix issue #1117.

[Copilot]

Pull request overview

This PR extends FGTable to support lookup tables with more than 3 dimensions by introducing an N-dimensional parsing/evaluation path based on nested <tableData breakPoint="..."> elements, and updates related schema, examples, and unit tests.

Changes:

• Generalize FGTable parsing and interpolation from fixed 1D/2D/3D to N-D via nested <tableData> breakpoints.
• Add public APIs for 4D–6D lookups plus a vector-based lookup overload.
• Update XML schema and add new unit tests and an example aircraft configuration demonstrating ND table syntax.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
src/math/FGTable.cpp	Implements ND XML parsing, breakpoint nesting, and recursive interpolation for N-D tables.
src/math/FGTable.h	Updates table API/documentation to expose ND lookup overloads and store lookup axes dynamically.
tests/unit_tests/FGTableTest.h	Adds basic load tests for 4D/5D/6D XML table structures.
JSBSimCommon.xsd	Extends schema to allow recursively nested <tableData> for ND tables and adds missing type attribute.
aircraft/ballx/ballx.xml	Provides a concrete 4D table example within an aircraft config.
aircraft/ballx/reset00.xml	Adds an initialization preset for the new example aircraft.
.gitignore	Adds ignores for devcontainer and environment/venv files.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The new recursive table-data type is mixed="true" and no longer constrains leaf <tableData> content to numbers-list, so schema validation will now accept empty/non-numeric <tableData> that JSBSim rejects at runtime. If possible, consider a stricter schema (e.g., XSD 1.1 xs:assert, or separate leaf/container types) or at least document this loss of validation explicitly.

[agodemar]

@bcoconni thank you for reviewing this PR.
I'm not sure you also see the additional reviews that GitHub Copilot provides after yours, or I'm the only one that sees them because of my licenses.
I kindly ask you now what I am supposed to do at this point?

• Beneath your diffs, the GitHub interface gives me a button "Commit suggestion". If I commit, does it overwrite your pushes?
• What happens to my branch? I see here master...agodemar:jsbsim:n-dimensional-interpolation two commits today from @bcoconni but where are the other adjustment you did?

[bcoconni]

@bcoconni thank you for reviewing this PR. I'm not sure you also see the additional reviews that GitHub Copilot provides after yours, or I'm the only one that sees them because of my licenses.

Yes, I can see them as well, and I guess everyone can.

Just for the record, I have pushed the button because I have some premium requests left on my Pro plan and I know I will not be able to use all of them before the reset on May 1st 😄

I kindly ask you now what I am supposed to do at this point?

• Beneath your diffs, the GitHub interface gives me a button "Commit suggestion". If I commit, does it overwrite your pushes?

If you push the button "Commit suggestion", it is supposed to apply on the latest commit in your branch which includes the ones I have pushed earlier.

• What happens to my branch? I see here master...agodemar:jsbsim:n-dimensional-interpolation two commits today from @bcoconni but where are the other adjustment you did?

Yes, sorry I took the liberty to push directly on the PR branch because they were trivial changes that restored the CI workflow. I needed that to check that everything was working in your branch. Apologies for that.

I cannot modify (nor anyone else) anything in your repo beyond the branch that you pushed for this PR. If you do not want other maintainers from JSBSim to push on branches you submit for PRs, you need to untick the option "Allow edits and access to secrets by maintainers" next to the "Create Pull Request" button:

You should be able to restore everything on your side with git pull origin n-dimensional-interpolation while your local branch n-dimensional-interpolation is checked out (i.e. active). If neede, replace origin with the name of the remote you are using

Once again, apologies for the confusion that it brought.

[agodemar]

@bcoconni no problem, nothing to apologize for. I'm just trying to understand who's doing what.
I did check on purpose the option "Allow edits and access to secrets by maintainers" because I really do rely on your expertise. After all, our interest here is to converge faster towards this new feature.

Regarding the suggestions by Copilot, I'll accepr them. In our unit tests we are only proving that parsing the tables succeeds. We should also assert at least that one expected lookup result (exact-grid and interpolated) to exercise the added ND lookup path.

[bcoconni]

Regarding the suggestions by Copilot, I'll accepr them. In our unit tests we are only proving that parsing the tables succeeds. We should also assert at least that one expected lookup result (exact-grid and interpolated) to exercise the added ND lookup path.

Looks good to me.

When you will do that, please use the button "Add suggestion to batch" to include all the suggestions in one single commit and avoid launching as many CI jobs as there are suggestions.

[bcoconni]

I have pushed some commits to this PR:

• Fixed 2 suggestions made by Copilot (exceptions from std::stoul and repeated allocations on the heap).
• Fixed some errors introduced by Copilot suggestions: it suggested to remove variables (a1, a2, ...) creation but also made some suggestions to use them to test FGTable::GetValue().
  • Lesson learnt: Copilot do not check the consistency of its suggestions.
• FGTable::GetValue(const std::vector<double>& keys) now throws if the size of its input argument keys is smaller than the dimension of the table as per the discussion Lookup tables in more than 3 dimensions #1425 (reply in thread)

[bcoconni]

Looks good to me.

[agodemar]

I added the Jupyter notebook examples/python/test_ballx.ipynb to experiment with the 4D interpolation.

At the moment the result doesn't look correct. In the bottom-right plot, the red dot should be on the gray surface, but it's not:

There's probably something wrong with the function jsbsim_lookup4D defined in the notebook.

@seanmcleod70 @bcoconni ideas?

[agodemar]

@seanmcleod70 @bcoconni did you have the chance to look at my notebook examples/python/test_ballx.ipynb yet?

[seanmcleod70]

I did briefly look at it at the time you posted, but didn't spot anything obviously wrong, and had a couple of questions, but then got distracted by the JOSS paper etc. 😉

    # Interpolated lookup at row=1, col=1 for the current (axis3, axis4) slice.
    x0, y0 = 1.0, 1.0
    ax3 = 20.0
    ax4 = 1.0
    z_ = float(np.asarray(jsbsim_lookup4D(row=x0, col=y0, axis3=ax3, axis4=ax4)))
    #z_=5.0 # just to test
    ax.scatter([x0], [y0], [z_], color="red", s=120, edgecolor="k", linewidth=0.7)

    ax.text(x0, y0, z_, f"  ({x0:.1f}, {y0:.1f}, {ax3:.1f}, {ax4:.1f}; {z_:.3f})", color="red", fontsize=9, weight="bold")

So you mention ..for current (axis3, axis4) slice, but it looks like all four of the axes are hard-coded (1.0, 1.0, 20.0, 1.0) for which you get a lookup value of 0.28? And the lookup value should be on the order of 10.x if I eyeball the dummy4 table?

[seanmcleod70]

Ah, shouldn't this:

def jsbsim_lookup4D(row, col, axis3=0.0, axis4=0.0):
    fdm['aero/dummy4D/aero/alpha-deg'] = row
    fdm['aero/dummy4D/aero/beta-deg'] = col
    fdm['aero/dummy4D/fcs/flap-pos-deg'] = axis3
    fdm['aero/dummy4D/fcs/parachute_reef_pos_norm'] = axis4

Rather be:

def jsbsim_lookup4D(row, col, axis3=0.0, axis4=0.0):
    fdm['aero/alpha-deg'] = row
    fdm['aero/beta-deg'] = col
    fdm['fcs/flap-pos-deg'] = axis3
    fdm['fcs/parachute_reef_pos_norm'] = axis4

In other words I don't think you were actually setting the relevant properties, and your lookup was for (0, 0, 0, 0), which is exactly 0.28.

[agodemar]

@seanmcleod70 oh, thanks!

[agodemar]

@bcoconni @seanmcleod70
I think we got to the point we could merge this PR.

I fixed the example examples/python/test_ballx.ipynb that gives this output when I test the 4D-interpolation on data provided in ballx.xml example:

The two bigger dots, colored in red and magenta, look correctly placed onto the light blue surfaces, in the right-top and right-bottom subplots.

What do you think?

PS: Some days ago, I configured the hook betweeen JSBSim releases and the Zenodo artifact uploads. I kindly ask @bcoconni to double check if the settings is OK. If we decide to merge this PR, it would be a good chance to make a minor release and see if the artifacts are correctly uploaded to Zenodo.
Bear in mind that DOI of our Zenodo artifacts are going to be cited in our CITATION.cff file.

[bcoconni]

I kindly ask @bcoconni to double check if the settings is OK.

@agodemar Oops ! Sorry, I have missed that. Could you remind me what was that I had to check ?

I think we got to the point we could merge this PR.

Yep, still looking good to me.

[agodemar]

I kindly ask @bcoconni to double check if the settings is OK.

@agodemar Oops ! Sorry, I have missed that. Could you remind me what was that I had to check ?

I think we got to the point we could merge this PR.

Yep, still looking good to me.

Oh, never mind. It's a setting that I can configure from Zenodo when I log with my GitHub credentials.

Here is what I have now:

When a new release comes out, I expect tha Zenodo will emit a badge and the DOI that we can use later on.

[agodemar]

@seanmcleod70 are you OK with merging this PR?

[seanmcleod70]

are you OK with merging this PR?

Yes.

[bcoconni]

Great! The PR is now merged.

[seanmcleod70]

@agodemar I meant to ask, what are the particular dimensions you're looking at for the high dimensional data you need to use?

[agodemar]

@agodemar I meant to ask, what are the particular dimensions you're looking at for the high dimensional data you need to use?

I'm supervising a project work where some provided lookup tables are 5D and 6D. Not more than that.

[seanmcleod70]

I should've been clearer, I was looking for a list of parameters for the dimensions, e.g. (alpha, beta, flaps, speedbrake, aileron) for force/moment X.