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

Added further tests to CI and provide Dockerimage with builds to swiftshader #119

Merged
merged 23 commits into from
Jan 24, 2021

Conversation

axsaucedo
Copy link
Member

@axsaucedo axsaucedo commented Jan 24, 2021

Extended work on #114

PR includes:

  • Docker image for building Kompute related work with all dependencies
  • Further test coverage in github action for C++ tests
  • Coverage for (some initial) Pyhton tests via separate action
  • Added Numpy as Setup.py dependency (potentially should explore making it optional)
  • Added Python dependencies to run tests

Outstanding (to cover in PR or open respective issues):

  • Documentation
  • Running further Python tests
  • Running further C++ tests
  • Make Numpy dependency optional - For now we'll keep it as required dependency

@axsaucedo axsaucedo added the enhancement New feature or request label Jan 24, 2021
@alexander-g
Copy link
Contributor

Consider adding pyshaderc to requirements-dev.txt and compiling GLSL in the tests:
spirv = pyshaderc.compile_into_spirv(glsl_bytes, 'comp', filepath='')

@axsaucedo
Copy link
Member Author

Interesting @alexander-g, good idea, that could provide full coverage of the tests, let me give it a shot

@alexander-g
Copy link
Contributor

Make Numpy dependency optional

What's the reasoning behind this? Numpy is de-facto standard in anything computing related.

@axsaucedo
Copy link
Member Author

@alexander-g the module was failing to import if the numpy dependency was not present, so I added it to setup.py - it's mainly to assess whether there would be users who would like to install Kompute without numpy to minimise dependencies. I do agree that numpy is the defacto standard in python so should make sense to have it by default.

@axsaucedo
Copy link
Member Author

Just looking at PyShaderc it doesn't look like it's currently maintained (no updates since 2017), so not sure adding it as a dependency. I am continuously surprised the lack of tools / utilities around GPGPU workflows, particularly around Python - let me give it some thought, as I would rather take in something along the lines of a simple GLSL utility function that is limited but maintained than bringing in packages that don't have too much support. Thinking outloud on this, I had a few interactions with the author of https://github.com/pygfx/pyshader - it may actually be a good opportunity for them to adopt some of these functionalities.

@alexander-g
Copy link
Contributor

Yes it's not maintained any more but it's supposed to be only a dependency for tests, not for the end user.

@axsaucedo
Copy link
Member Author

That's technically correct, but currently large number of users would use the tests for guidance on how to use the library (that's largely how pybind11 provides insights to more advanced usage, by pointing users to the unit tests) - having said that, I think if there is a way to explore a more active project like pyshader to offer these functionalities. Let me reach out to the pyshader project, as that would be the ideal, but if that's not an option I think pyshaderc is still a good choice to get further coverage of tests.

@axsaucedo
Copy link
Member Author

Updated docs https://kompute.cc/overview/ci-tests.html

@axsaucedo
Copy link
Member Author

Added #121 to ensure the rest of the tests are covered

@axsaucedo axsaucedo merged commit 7af493e into master Jan 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants