Unit testing

[gipert]

Here's a to-do list, I'll kep it up-to-date. Good reference: https://scikit-hep.org/developer. Most of this will be implemented on the refactor branch only.

• [ ] Add micro-tests for each module/function separately. Read the contribution guide (refactor branch). Add @gipert as a reviewer for your pull requests. Assignments (put your name!):
  • dsp @iguinn
  • flow
  • hit @wisecg
  • lgdo @jasondet
  • math
  • pargen @ggmarshall
  • raw @jasondet
  • vis @iguinn
• [ ] Develop/import synthetic pulse injector
• [ ] Add high-level tests of the full production chain by using real data from https://github.com/legend-exp/legend-testdata @gipert, ...
  • Include all necessary data in legend-testdata
• [x] Let a Python linter run as part of tests to enforce uniform coding conventions (see also #242)
• [x] Add test coverage reporting through https://codecov.io and GitHub bot

[jasondet]

test installation doesn't work for me:

pygama % pip install .[test]
zsh: no matches found: .[test]

Also just running pytests fails:

pygama % pytest
ERROR: usage: pytest [options] [file_or_dir] [file_or_dir] [...]
pytest: error: unrecognized arguments: --cov=pygama
  inifile: /Users/jasondet/Work/Coding/pygama/pyproject.toml
  rootdir: /Users/jasondet/Work/Coding/pygama

@gipert can you take a look?

[gipert]

This is zsh... you need to wrap [] in single quotes, like this: pip install '.[test]'.

I've forgotten zsh is now the default shell on OSX.

[jasondet]

ah that works! can you update the instructions?

incidentally, using the single quotes also works in bash... maybe make that the instruction?

also, the instructions should specify where to start from (i.e. .'[test]' inside the pygama dir, pygama.'[test]' from the dir containing pygama)

[slwatkins]

What is the plan for unit tests and code coverage for numba-ized functions? (e.g. the processors) We can write unit tests, but because of the JIT compilation, the actual code itself is not counted as covered (see https://github.com/numba/numba/issues/4268).

Should we exclude these functions from the overall coverage? We should still write tests for them of course, but it doesn't seem like we would ever be able to get pytest to mark these blocks of code as "covered".

[gipert]

Good point @slwatkins. What about disabling JIT in unit tests? Would that fix the problem?

[iguinn]

It looks like their attempt to solve this in numba has stalled out a bit (no updates to this pull request https://github.com/numba/numba/pull/5660 since 2020). Their recommendation seems to be disabling JIT using the environment variable, but that solution worries me because of the possibility that pure numpy ends up working differently from numba guvectorize. It is possible to access the underlying python function without a decorator by using __wrapped__: https://stackoverflow.com/questions/63512189/can-i-run-a-decorated-function-without-a-decorator-functionality (this solution doesn't necessarily work for every kind of decorated function, but it does seem to work for numba):

In [1]: from pygama.dsp.processors import trap_filter
In [2]: import numpy as np
In [3]: trap_filter.__wrapped__(np.ones(20), 2, 2, np.zeros(20))

We should see if adding a test on the __wrapped__ function triggers the coverage. Note that the guvectorized ability to call without including the output variable as an argument will not work here (relevant for how the tests are written so far).

[slwatkins]

It looks like using __wrapped__ works well, as shown in #399. Right now, it requires the test-writer to effectively duplicate each line of code (we want to test that both the python and the numba versions have the same answers). Is there a more elegant solution?

Perhaps we can write a generic wrapper for a function that calculates the value using the numba and python versions, asserts that they are equal (or up to floating precision), and then returns the value for the test to run?

[slwatkins]

I just came across the inspect.unwrap function, perhaps this would be the way to go instead of using __wrapped__?

https://docs.python.org/3/library/inspect.html#inspect.unwrap

[slwatkins]

I took a stab at something generic for testing numba versus python for the processors, the code is ugly right now (and the variables should probably be renamed), but it seems to work for the numbafied processors that have just the decorator (as opposed to functions like cusp_filter)

import re
import inspect
import numpy as np

def numba_vs_python(func, *inputs):

    # get outputs
    all_params = list(inspect.signature(func).parameters)
    output_sizes = re.findall('(\(n*\))', (func.signature.split('->')[-1]))
    noutputs = len(output_sizes)
    output_names = all_params[-noutputs:]

    # numba outputs
    outputs = func(*inputs)
    if noutputs==1:
        outputs = [outputs]

    # unwrapped python outputs
    func_unwrapped = inspect.unwrap(func)
    output_dict = {
        key: np.empty(len(inputs[0])) for key in output_names
    }
    func_unwrapped(*inputs, **output_dict)
    for spec, key in zip(output_sizes, output_dict):
        if spec == '()':
            output_dict[key] = output_dict[key][0]
    output_vals = [output_dict[key] for key in output_names]

    # assert that numba and purepython should be the same (maybe use np.isclose)
    assert all(all(np.atleast_1d(o1) == np.atleast_1d(o2)) for o1, o2 in zip(output_vals, outputs))

    # return value for comparison with expected solution
    return outputs[0] if noutputs == 1 else outputs

[gipert]

Thumbs up! Can you open a PR with this? Maybe we merge #399 first.

[slwatkins]

I'll clean it up a little bit, and open a PR - I can mark #399 as ready, and maybe we can update that test when we're happy with how we're testing these numba functions?

[gipert]

Agreed.