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

Add function to run both the numba and pure python versions of some function #402

Merged

Conversation

slwatkins
Copy link
Member

As discussed in #241 and proposed in #241 (comment), we can write a function that takes a numba-wrapped function, and automatically runs both the numba version and the unwrapped pure python version. For testing these numba-wrapped functions, it is necessary to run the pure python version to trigger the code coverage.

I've added a fixture to tests/conftest.py, which returns such a function for some generic wrapped function in, e.g., dsp. I've also updated the fixed_pickoff_time test to use this function as an example, which cleans up the test substantially.

I'll keep this as a draft for now, as I'm curious if there's a better place to store this function or if there are any thoughts on how I'm using it. I also want to point out that it doesn't work well with pytest.raises, so I'm just using inspect.unwrap for checking if the errors are being raised for both cases.


Before submitting a pull request, please make sure you've read and understood the pygama developer's guide: https://pygama.readthedocs.io/en/latest/developer.html. In particular, do not forget to:

  • Conform to our coding conventions
  • Update existing or add new tests
  • Update existing or add new documentation
  • Address any issue reported by GitHub checks

@gipert
Copy link
Member

gipert commented Oct 26, 2022

Like! Can be merged from my point of view, after adding a docstring and a note to the dev's guide. I am curious to see whether vectorization will yield results which differ from the pure python implementation by more than machine's precision.

@gipert gipert added tests Testing the package dsp Digital Signal Processing labels Oct 26, 2022
@gipert gipert requested a review from iguinn October 26, 2022 08:47
@slwatkins slwatkins marked this pull request as ready for review October 27, 2022 15:43
@slwatkins
Copy link
Member Author

@gipert I added some documentation on using it, and I think it should be ready to go - let me know if there's any follow up comments on what I've changed!

@jasondet
Copy link
Collaborator

This is awesome @slwatkins thanks! I'll merge it once the checks are complete

@gipert gipert merged commit f93ce7a into legend-exp:main Nov 2, 2022
@slwatkins slwatkins deleted the tests/autocheck_numba_and_python_outputs branch December 7, 2022 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dsp Digital Signal Processing tests Testing the package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants