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

remove need for supportData folder in BSK wheels #814

Open
schaubh opened this issue Sep 21, 2024 · 3 comments
Open

remove need for supportData folder in BSK wheels #814

schaubh opened this issue Sep 21, 2024 · 3 comments
Assignees
Labels
build Build system or compilation enhancement documentation Improvements or additions to documentation enhancement New feature or request

Comments

@schaubh
Copy link
Contributor

schaubh commented Sep 21, 2024

Describe your use case
Right now building a wheel includes the large Spice files in supportData folder. These are really only needed when running some scenario scripts. Instead, I propose that this folder be moved to inside the examples folder. This should reduce greatly the BSK wheel size, making it easier to host on PyPi among other things.

If users want to run the examples scripts and download this folder, they would get the required files except for the large Spice files. These would have to be manually downloaded if the user is not building BSK using cmake which would download them. I think this is manageable with good documentation and warning messages if the file is not found and can't be loaded.

Describe alternatives solutions you've considered
I can't think of any alternatives right now, open to ideas.

Additional context
The support data is not explicitly required by any module to compile. What data to load can be set by the user and we are providing handy defaults options. I'll have to play with a test branch to see what unexpected challenges arise from this approach.

@schaubh schaubh added documentation Improvements or additions to documentation enhancement New feature or request build Build system or compilation enhancement labels Sep 21, 2024
@schaubh schaubh self-assigned this Sep 21, 2024
@schaubh
Copy link
Contributor Author

schaubh commented Sep 21, 2024

Howdy @dpad , I have been able to expand the CI test actions to now cover a range of python and Linux versions, as well as Window with opNav, and macOS with opNav and one action on macOS without vizInterface. The macOS with opNav script also test builds the documentation. My next step is to try to remove the need to have the dataSupport move to the examples folder and not be included in the BSK build per say. I'll see if I have time to play with this idea on a branch this weekend. Do you see any issues with your build method with this approach. Or, is there an impact on upgrading our use of conan from 1.0 to 2.0? In your other branch you already had 2.0 functionality working.

@dpad
Copy link
Contributor

dpad commented Sep 26, 2024

@schaubh Sorry about the delayed response, I am currently traveling.

I think there's no issue in upgrading conan to 2.0, as you say I had it working on the other branch. I think there was only some minor issues in versions and options for the dependencies that we specify in the conanfile, but I just had to pin those to an appropriate working version.

Regarding supportData, yes, if we only need the data for examples, then they don't need to be included in the built wheel. I don't think there would be any issues with the build system, we would just need to change which files get included in the appropriate pyproject.toml settings. The issues would only be during usage I think (e.g. what happens if a file that Basilisk expects is missing, should it give an error at run-time or initially during configuration of the simulation, should we provide a method to automatically download the data, and if so from where, and what happens if there are networking issues, etc.)

Regarding wheel size, one thing I noticed is that there's a lot of duplication in the compiled module files (I think because of the way we essentially copy-paste the messaging library code instead of linking to a shared library, for example see the auto-generated .cxx files for messages). I mentioned before that we should use cibuildwheel to create wheels compatible across lots of different systems at once. One of the things this does is to run auditwheel repair on the wheel file to check and fix up the compatibility of the wheel. I realised that you can run auditwheel repair --strip on the wheels to remove a bunch of unused symbols from the compiled module files (the .so files) -- when I was testing this it reduced the total wheel size to less than half.

@schaubh
Copy link
Contributor Author

schaubh commented Sep 26, 2024

Thanks for the info. I'll looking into the SupportData ideas over the next weeks, and good to know about the audit wheel repair --strip suggestion. I'll try that. That might get us closer already to our target of having a BSK wheel that is less than 100Mb if possible. I'm on travel a lot over the next 3 weeks, so my productivity will be a little slower ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Build system or compilation enhancement documentation Improvements or additions to documentation enhancement New feature or request
Projects
Status: No status
Development

No branches or pull requests

2 participants