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

A/C data file #337

Closed
galleon opened this issue Oct 22, 2020 · 29 comments
Closed

A/C data file #337

galleon opened this issue Oct 22, 2020 · 29 comments

Comments

@galleon
Copy link
Contributor

galleon commented Oct 22, 2020

Thanks a lot for creating wheels for mac, linux & windows platform. I will use them in gym-jsbsim.
I have noticed that there is a new release since mid of October.

In order to be able to run the wheels, we need the data for the aircraft?

Would that make sense to create for each release a dedicated data asset file that would contain the files needed to run the wheels?

For the time being the README redirects to the Release Candidate (i.e. 1.0.0) not taking into account possible update and/or new file?

I can propose a PR if that if judged useful

BTW I would need to understand if the aircraft and engine directories are the only one needed to operate existing a/c?

thanks
Guillaume

@seanmcleod
Copy link
Member

The source code links, e.g. https://github.com/JSBSim-Team/jsbsim/archive/v1.1.0.zip includes all the required data for the included aircraft etc.

In addition to the aircraft and engine directories you will potentially also need the systems directory.

@galleon
Copy link
Contributor Author

galleon commented Oct 22, 2020

Thanks Sean for clarifying the required directories

Ok so it might make sense to update the README to point to the latest file.
On top of that do we want to have a dedicated file for only the data files or are we happy to just point to a superset (i.e. the source archive) ?

@seanmcleod
Copy link
Member

The README has the following link - JSBSim project release section which has the latest release at the top of the page. Do you think we really need to include and update the link in the README to the latest release, e.g. to https://github.com/JSBSim-Team/jsbsim/releases/tag/v1.1.0?

Yep, up until now we've pointed people to the source code zip file for aircraft data. If we did break out a separate data zip file it would probably also make sense to add the scripts directory to it as well.

I see the README actually specifically mentions aircraft data being available in the source code zip file, although the example link is to a specific older version - https://github.com/JSBSim-Team/jsbsim/archive/Release_Candidate_v1.0.0.zip

@bcoconni
Copy link
Member

For the time being the README redirects to the Release Candidate (i.e. 1.0.0) not taking into account possible update and/or new file?

The README file is modified manually and has not been updated following release 1.1.0 unfortunately.

Ok so it might make sense to update the README to point to the latest file.

Indeed.

If we did break out a separate data zip file it would probably also make sense to add the scripts directory to it as well.

Yes and the scripts and data_output folders should be of interest too. The former contains all the scripts and the latter contains the file flightgear.xml which are the settings to use FlightGear has an external visualizer.

On top of that do we want to have a dedicated file for only the data files or are we happy to just point to a superset (i.e. the source archive) ?

I agree it makes sense to have a dedicated zip file for data files. The workflow cpp-python-build.yml should be updated to produce a package of the data files that match the binaries & wheels release.

I am also considering using Inno Setup to build a Windows installer that would contain the executables JSBSim.exe, aeromatic.exe and the data files. What do both of you think ?

Inno Setup is an open source tool and is installed by default on GitHub servers.

@galleon
Copy link
Contributor Author

galleon commented Oct 23, 2020

I have created #338 to address the README update whilst this is not critical.

Thinking out loud, instead of creating a dedicated zip file one could think of adding the folders in the wheels directly using package_data. On my side I always rely on the a/c data from this repo but I don't know what other users do ?
Is a wheel without a/c data useful ? Is the data in the deb packages ?

Providing a windows installer using InnoSetup is definitely a good idea and with lower again the effort to use JSBSim

@bcoconni
Copy link
Member

bcoconni commented Oct 23, 2020

Thinking out loud, instead of creating a dedicated zip file one could think of adding the folders in the wheels directly using package_data. On my side I always rely on the a/c data from this repo but I don't know what other users do ?

I have considered that option but the data should be installed at a location where the user is able to retrieve them. Said in other words, where the wheel package should install the package ?

Is a wheel without a/c data useful ?

I guess that for a power user that develops his/her own aircraft or application the answer is yes. On the other hand, a new user will most likely download the aircraft data to practice with JSBSim.

Is the data in the deb packages ?

No, for the same reason than wheel packages do not provide the aircraft data folders (i.e. where the data should be installed ?)

Providing a windows installer using InnoSetup is definitely a good idea and with lower again the effort to use JSBSim

Yes and an additional benefit is that installers GUI allow user to select the path where the aircraft data will be installed.

@seanmcleod
Copy link
Member

Yes and an additional benefit is that installers GUI allow user to select the path where the aircraft data will be installed.

Yep the installer's GUI could also prompt the user to select what they would like to install, e.g.

  • Binaries
  • Data
  • Source code

In addition to where they would like to install each of them.

bcoconni added a commit that referenced this issue Oct 23, 2020
As per the discussion in issue #337, this commit includes executables and aircraft data in the installer.
@bcoconni
Copy link
Member

Since commit 9ddd718, we are providing a Windows installer JSBSim-1.2.0.dev1-setup.exe that is available in the rolling release at the moment. This replaces the raw executables JSBSim.exe and aeromatic.exe that were provided until now.

As per @seanmcleod suggestion, users are prompted to install binaries and/or aircraft data.

At the moment, the installer does not include the source code. I guess that at some point it will provide JSBSim headers and library rather than the whole source code. Indeed if one would want to install the complete source code I would rather expect that git would be used or the source code archive downloaded (either .zip or .tar.gz).

Please note that the MSVC C++ dll msvcp140.dll is included in the installer so that executables can be used even if MSVC++ is not installed. The dll is installed in the same folder than the executables rather than systemwide because I did not want to be caught in the turmoil of the Windows management of the system DLLs. If someone knows better, patches are welcome 😉

bcoconni added a commit that referenced this issue Jan 23, 2021
Each time a new stable version is released, the corresponding references in README.md must be updated. Manual update is error prone not considering that it might be overlooked by maintainers until someone opens an issue (see issue #337 and PR #338).

This commit fixes that and automatically pushes changes to README.md to update all the references when a new stable release is built.
bcoconni added a commit that referenced this issue Jan 23, 2021
Each time a new stable version is released, the corresponding references in README.md must be updated. Manual update is error prone not considering that it might be overlooked by maintainers until someone opens an issue (see issue #337 and PR #338).

This commit fixes that and automatically pushes changes to README.md to update all the references when a new stable release is built.
@bcoconni
Copy link
Member

For the time being the README redirects to the Release Candidate (i.e. 1.0.0) not taking into account possible update and/or new file?

Just a heads up: I have just pushed the commit ac8fbe2 which automatically updates README.md with the references/links to the last stable release.

I realized by chance that I had forgotten to update these references for version 1.1.3 while I was updating the list of JSBSim users in README.md. Now this should no longer happen since our CI workflow will automatically take care of that.

Reminder to @JSBSim-Team: new releases are triggered by pushing a tag to the git repo. Our CI workflow then automatically builds all the packages/executables, updates the documentation and creates a new release.

bcoconni added a commit that referenced this issue Nov 7, 2021
Default JSBSim aircraft data (including its engine and systems data) and scripts are now included in the Python wheels.

The files are installed in the sys.prefix folder under share/JSBSim.
bcoconni added a commit to bcoconni/jsbsim that referenced this issue Nov 7, 2021
Default JSBSim aircraft data (including its engine and systems data) and scripts are now included in the Python wheels.

The files are installed in the sys.prefix folder under share/JSBSim.
@bcoconni
Copy link
Member

Thinking out loud, instead of creating a dedicated zip file one could think of adding the folders in the wheels directly using package_data. On my side I always rely on the a/c data from this repo but I don't know what other users do ? Is a wheel without a/c data useful ? Is the data in the deb packages ?

Starting from release 1.1.9 that has been released a few days ago, wheels and conda packages are now including the complete set of JSBSim aircraft data and example scripts.

If you declare the FGFDMExec root parameter as None then the aircraft, engine and systems paths will point to the default aircraft data. So the following script will work out of the box just after pip install or conda install have been run:

import jsbsim

fdm = jsbsim.FGFDMExec(None)
fdm.load_script('scripts/c1723.xml')
fdm.run_ic()

while fdm.run():
    pass

And the path to the aircraft data can easily be retrieved with:

print(jsbsim.get_default_root_dir())

@seanmcleod
Copy link
Member

@bcoconni that'll be useful for quick easy setup. I know previously when I installed JSBSim into a cloud based Jupyter notebook it was a bit of a pain to get the JSBSim data into the virtual machine etc.

Just tried it out with Google's Colab, but ran into an issue with the JSBSim data.

Not clear if there is a permission issue and the pip install jsbsim is silently failing to create and populate the data directory?

image

@seanmcleod
Copy link
Member

Trying it locally on my Windows PC it worked.

(base) C:\Users\Sean>pip install jsbsim
Collecting jsbsim
  Downloading JSBSim-1.1.9-623-cp37-cp37m-win_amd64.whl (1.4 MB)
     |████████████████████████████████| 1.4 MB 3.3 MB/s
Requirement already satisfied: numpy in c:\users\sean\anaconda3\lib\site-packages (from jsbsim) (1.18.1)
Installing collected packages: jsbsim
Successfully installed jsbsim-1.1.9
(base) C:\Users\Sean>python
Python 3.7.6 (default, Jan  8 2020, 20:23:39) [MSC v.1916 64 bit (AMD64)] :: Anaconda, Inc. on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import jsbsim
>>> print(jsbsim.get_default_root_dir())
C:\Users\Sean\anaconda3\share\JSBSim
>>> fdm = jsbsim.FGFDMExec(None)


     JSBSim Flight Dynamics Model v1.1.9 [GitHub build 623/commit 56bb9b9cfbcd06d80afd682cea42b9d049488824] Nov  7 2021 18:44:16
            [JSBSim-ML v2.0]

JSBSim startup beginning ...

@seanmcleod
Copy link
Member

Semi-random observation/question. Why is the Linux wheel 23.9MB and the Windows wheel is only 1.4MB?

@bcoconni
Copy link
Member

Semi-random observation/question. Why is the Linux wheel 23.9MB and the Windows wheel is only 1.4MB?

This is most likely due to the fact that Linux wheels bundle almost all the libraries that the package depends on. This is by design since there are a lot of flavors of Linux distribution. Extract from the auditwheel docs:

auditwheel repair: copies these external shared libraries into the wheel itself, and automatically modifies the appropriate RPATH entries such that these libraries will be picked up at runtime. This accomplishes a similar result as if the libraries had been statically linked without requiring changes to the build system. Packagers are advised that bundling, like static linking, may implicate copyright concerns.

@bcoconni
Copy link
Member

Not clear if there is a permission issue and the pip install jsbsim is silently failing to create and populate the data directory?

The package seems to have been installed in /usr/local. Have you checked /usr/local/share ?

@bcoconni
Copy link
Member

It seems that the data files can also be installed in the folder contained in site.USER_BASE:

import site

print(site.USER_BASE)

@galleon
Copy link
Contributor Author

galleon commented Nov 11, 2021 via email

@bcoconni
Copy link
Member

bcoconni commented Nov 11, 2021

It is indeed in /usr/local/share

Right. So we need to autodetect that.

From I could gather right now, the files can be located in either:

from distutils.dist import Distribution
from distutils.command.install import install
cmd = install(Distribution())
cmd.ensure_finalized()
path = cmd.install_lib

Also of interest: StackOverflow "How do I find the location of my Python site-packages directory?"

bcoconni added a commit to bcoconni/jsbsim that referenced this issue Nov 11, 2021
As reported in issue JSBSim-Team#337, the a/c data can be installed at a variety of location depending on the context of the installation (virtual envs, user install or global install).

This fix parses the default folders that are likely to contain JSBSim a/c data.
@bcoconni
Copy link
Member

So we need to autodetect that.

This should now be the case since commit 5665833.

@seanmcleod
Copy link
Member

This should now be the case since commit 5665833.

The commit was roughly 15hrs ago, but it doesn't look like the python wheels have been updated and pushed yet? Just tried again on Google colab with a fresh pip install and it's still failing in the same way.

@bcoconni
Copy link
Member

No sorry, I have not triggered the wheels generation yet. I'd like someone to compile the wheels themselves and check that it works before triggering a release.

@galleon
Copy link
Contributor Author

galleon commented Nov 12, 2021

I did test again on colab using !pip install --pre --find-links ./ jsbsim after downloading this wheel

https://github.com/JSBSim-Team/jsbsim/releases/download/Linux/JSBSim-1.2.0.dev1-cp37-cp37m-manylinux_2_12_x86_64.manylinux2010_x86_64.whl

And everything went well on that platform

@bcoconni
Copy link
Member

And everything went well on that platform.

Superb ! 👍

bcoconni added a commit to bcoconni/jsbsim that referenced this issue Nov 12, 2021
As reported in issue JSBSim-Team#337, the a/c data can be installed at a variety of location depending on the context of the installation (virtual envs, user install or global install).

This fix parses the default folders that are likely to contain JSBSim a/c data.
@bcoconni
Copy link
Member

JSBSim version 1.1.10 has just been released. It contains the fix for the error in locating the directory that contains aircraft data. I have checked both locally and one Google Colab and it worked in both cases.

If you can confirm that it works for you then I'd suggest to close this issue.

@seanmcleod
Copy link
Member

Just tested 1.1.10 in my Google Colab notebook and locally on Windows 11, both worked as expected.

One thing I did notice when running the script is that the .csv output file ends up in share\JSBSim, and doing a pip uninstall jsbsim doesn't clean them up.

(base) C:\Users\Sean>dir .\anaconda3\share\JSBSim\ -recurse


    Directory: C:\Users\Sean\anaconda3\share\JSBSim


Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
d-----        11/12/2021  11:32 PM                aircraft
-a----        11/12/2021  11:30 PM        4182330 JSBout172B.csv


    Directory: C:\Users\Sean\anaconda3\share\JSBSim\aircraft


Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
d-----        11/12/2021  11:32 PM                c172x


    Directory: C:\Users\Sean\anaconda3\share\JSBSim\aircraft\c172x


Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
-a----        11/12/2021  11:30 PM            694 initfile.133.441667.xml

In general I guess you don't want an uninstall to delete any user generated data, just seems potentially messy leaving these output files around.

bcoconni added a commit to bcoconni/jsbsim that referenced this issue Nov 13, 2021
When running the script `c1723.xml` with the Python wheel code, the `.csv` output file ends up in `share\JSBSim`, and doing a `pip uninstall jsbsim` doesn't clean them up.

Following this commit, JSBSim has now a path to the directory where output files will be written. This path can therefore be different than the root directory. Two new methods ahve been added to the C++ class `FGFDMExec`: `SetOutputPath` and `GetOutputPath` (the corresponding Python methods are `set_output_path` and `get_output_path`).

Regarding the Python wheel, the output path is set to the current directory (i.e. `$PWD`) when using the bundled a/c data. In all likelyhood this should be different than the root directory.

The executable `JSBSim.exe` still uses the root directory as the output directory by default as it has always done.
@bcoconni
Copy link
Member

One thing I did notice when running the script is that the .csv output file ends up in share\JSBSim, and doing a pip uninstall jsbsim doesn't clean them up.

Indeed. I pushed the commit 6203a00 that should fix that. Basically, the idea is to allow output files to be written at a different place than the aircraft data location. This is obtained by 2 new C++ methods FGFDMExec::SetOutputPath and FGFDMExec::GetOutputPath.

With these 2 new methods, I could modify the Python wheels to write the output files at the current directory (i.e. $PWD) when using the bundled aircraft data and scripts. This is, of course, assuming that the current directory is different than the aircraft directory which I think is likely to be the case when using the bundled aircraft data. For all other cases, the output path points to the aircraft data directory as it always did. Of course this default setting can be overridden by calling FGFDMExec::SetOutputPath (C++) or FGFDMExec.set_output_path (Python).

Note that for some reason, pip uninstall still doesn't remove the folders share/JSBSim and share/JSBSim/aircraft but at least, they are empty now.

@bcoconni
Copy link
Member

I think I have found why the folders share/JSBSim and share/JSBSim/aircraft are not deleted: this may be because they do not contain package files but only subdirectories.

When uninstalling the package, I am suspecting an algorithm like below is executed:

  1. If there are package files in the current folder then delete them all.
  2. If as a result of step 1 the folder is empty then delete it.

Meaning that if the step 1 condition is not met, then the condition of step 2 is not evaluated.

bcoconni added a commit that referenced this issue Nov 21, 2021
Folders without files are not deleted when the wheel package is uninstalled (see issue #337). Add `LICENSE.txt` to the root folder and `aircraft_template.xml` (warning: it is outdated) to `aircraft` for a proper removel of the JSBSim Python package.
bcoconni added a commit that referenced this issue Nov 21, 2021
When running the script `c1723.xml` with the Python wheel code, the `.csv` output file ends up in `share\JSBSim`, and doing a `pip uninstall jsbsim` doesn't clean them up.

Following this commit, JSBSim has now a path to the directory where output files will be written. This path can therefore be different than the root directory. Two new methods ahve been added to the C++ class `FGFDMExec`: `SetOutputPath` and `GetOutputPath` (the corresponding Python methods are `set_output_path` and `get_output_path`).

Regarding the Python wheel, the output path is set to the current directory (i.e. `$PWD`) when using the bundled a/c data. In all likelyhood this should be different than the root directory.

The executable `JSBSim.exe` still uses the root directory as the output directory by default as it has always done.
bcoconni added a commit that referenced this issue Nov 21, 2021
Folders without files are not deleted when the wheel package is uninstalled (see issue #337). Add `LICENSE.txt` to the root folder and `aircraft_template.xml` (warning: it is outdated) to `aircraft` for a proper removel of the JSBSim Python package.
@bcoconni
Copy link
Member

bcoconni commented Nov 21, 2021

JSBSim should now clean up after itself after its Python package is uninstalled (commit 9b1b5cd).

As I was suspecting, folders need to contain at least one package file for being deleted upon removal. So I took this opportunity to include the LGPL license LICENSE.txt to the folder share/JSBSim. I have also included aircraft_template.xml to the folder share/JSBSim/aircraft. This one is more questionable since the XML format that it is using is obsolete.

Anyway, according to my tests, the inclusion of the 2 files mentioned above allows the proper removal of the whole folder share/JSBSim.

@bcoconni bcoconni mentioned this issue Nov 21, 2021
2 tasks
@bcoconni
Copy link
Member

Following the release of JSBSim v1.1.11, all the bugs reported in this issue have been fixed. This issue is closed. Please open a new issue if some further issues are encountered.

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

No branches or pull requests

3 participants