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

[JOSS] low and variable test coverage when running locally #32

Open
kandersolar opened this issue Aug 7, 2021 · 8 comments
Open

[JOSS] low and variable test coverage when running locally #32

kandersolar opened this issue Aug 7, 2021 · 8 comments
Assignees

Comments

@kandersolar
Copy link

This is more of a question than an issue report. When I run the test suite locally and calculate coverage, I get a much lower number than what the CI finds. The biggest coverage differences are rt.py and rcwa.py. The specific numbers also change from run to run (e.g. in another run, tmm.py showed 99% coverage). I'm happy to dig a bit more to try to understand why, but I wondered if this behavior is expected. Here's the result of $ coverage html: htmlcov.zip, and the corresponding test output:

$ pytest --cov-report= --cov=rayflare tests/

============================================= test session starts ==============================================
platform linux -- Python 3.8.10, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /home/kevin/projects/rayflare
plugins: cov-2.12.1
collected 33 items                                                                                             

tests/test_analytic.py .                                                                                 [  3%]
tests/test_compare_methods.py ............                                                               [ 39%]
tests/test_ideal_cases.py ...                                                                            [ 48%]
tests/test_matrix_formalism.py .                                                                         [ 51%]
tests/test_ray_tracing.py .                                                                              [ 54%]
tests/test_rigorous_coupled_wave.py ......                                                               [ 72%]
tests/test_textures.py ....                                                                              [ 84%]
tests/test_transfer_matrix_method.py .....                                                               [100%]

<warning summary removed>

$ coverage report
Name                                                  Stmts   Miss  Cover
-------------------------------------------------------------------------
rayflare/__init__.py                                      0      0   100%
rayflare/analytic/__init__.py                             0      0   100%
rayflare/analytic/diffraction.py                         63     22    65%
rayflare/angles.py                                       57     28    51%
rayflare/matrix_formalism/__init__.py                     2      0   100%
rayflare/matrix_formalism/ideal_cases.py                 48      4    92%
rayflare/matrix_formalism/multiply_matrices.py          261      7    97%
rayflare/matrix_formalism/process_structure.py           97      5    95%
rayflare/options.py                                      31      0   100%
rayflare/ray_tracing/__init__.py                          1      0   100%
rayflare/ray_tracing/rt.py                              613    484    21%
rayflare/rigorous_coupled_wave_analysis/__init__.py       1      0   100%
rayflare/rigorous_coupled_wave_analysis/rcwa.py         621    336    46%
rayflare/state.py                                        12      3    75%
rayflare/structure.py                                    40      5    88%
rayflare/textures/__init__.py                             2      0   100%
rayflare/textures/define_textures.py                     16      0   100%
rayflare/textures/standard_rt_textures.py                58     12    79%
rayflare/transfer_matrix_method/__init__.py               1      0   100%
rayflare/transfer_matrix_method/lookup_table.py          74     61    18%
rayflare/transfer_matrix_method/tmm.py                  294    146    50%
rayflare/utilities.py                                    24      4    83%
-------------------------------------------------------------------------
TOTAL                                                  2316   1117    52%

xref openjournals/joss-reviews#3460

@phoebe-p phoebe-p self-assigned this Aug 9, 2021
@phoebe-p
Copy link
Member

phoebe-p commented Aug 9, 2021

This is strange. I guess different test suites have slightly different ways of measuring coverage, and the 91% value that is reported on the RayFlare repository is calculated by codecov and not pytest, but the difference in total coverage (91 vs. 52%) seems very extreme, especially since codecov is calculating the coverage based on the test report generated by pytest. The fact that the coverage fluctuates is even stranger. The ray-tracing tests are obviously stochastic in nature but I can't think of a reason that would change the actual lines of code that get hit or not hit. Some tests have inputs which are randomly chosen from a range but again I cannot think of a reason why that would change which parts of the code are accessed. So basically, I have no idea why this is happening but I will try running pytest coverage locally on my own computer to see if I have the same problem.

@phoebe-p
Copy link
Member

phoebe-p commented Aug 9, 2021

I have just had a thought on why the RCWA coverage could be so different -- what OS are you running this on? For the GitHub Actions, because installing S4 is a pain and easiest on Linux, I am only installing it and running the tests associated with RCWA/S4 on Ubuntu, so they are marked to be skipped if the operating system is not Linux:

@mark.skipif(sys.platform != "linux", reason="S4 (RCWA) only installed for tests under Linux")

So if you delete these lines where they appear, it should run on your system (assuming S4 is installed).

I need to check to be sure but this may well also affect coverage of TMM and RT-related files, since there are some examples which compare the different methods to see if they give consistent results, and if a test compares e.g. RCWA and TMM it will currently only run on Ubuntu so could also affect TMM coverage. Obviously, this isn't very good practice for writing tests, I should really have more unit tests which isolate these things and test smaller bits of code separately, but it might explain part of the issue, though it does not explain the fact that the coverage changes.

@kandersolar
Copy link
Author

Ah sorry, I should have mentioned that I was running it on a linux (debian) installation with S4 installed. If skipif was the culprit here I think we'd see s characters instead of . in the pytest output, so I don't think that's the issue, but I'll double-check that all the test functions are really executing as expected. Could be that my virtual environment was messed up somehow, so I'll also start a fresh environment and see if that fixes things. Hopefully if nothing else, I should be able to follow the same steps that the CI performs and track down the problem, but it would still be interesting to see if you can replicate the issue locally or not.

@phoebe-p
Copy link
Member

phoebe-p commented Aug 9, 2021

Ok, running things locally on my computer also gives a much lower % of coverage than reported by codecov, but it's not variable between runs. Will have a look later to see if I can figure out why the coverage % for some files is so different (and which one is correct...).

---------- coverage: platform linux, python 3.8.10-final-0 -----------
Name                                                  Stmts   Miss  Cover
-------------------------------------------------------------------------
rayflare/__init__.py                                      0      0   100%
rayflare/analytic/__init__.py                             0      0   100%
rayflare/analytic/diffraction.py                         63     22    65%
rayflare/angles.py                                       57     28    51%
rayflare/matrix_formalism/__init__.py                     2      0   100%
rayflare/matrix_formalism/ideal_cases.py                 48      4    92%
rayflare/matrix_formalism/multiply_matrices.py          261      7    97%
rayflare/matrix_formalism/process_structure.py           97      5    95%
rayflare/options.py                                      31      0   100%
rayflare/ray_tracing/__init__.py                          1      0   100%
rayflare/ray_tracing/rt.py                              613    484    21%
rayflare/rigorous_coupled_wave_analysis/__init__.py       1      0   100%
rayflare/rigorous_coupled_wave_analysis/rcwa.py         621    372    40%
rayflare/state.py                                        12      3    75%
rayflare/structure.py                                    40      5    88%
rayflare/textures/__init__.py                             2      0   100%
rayflare/textures/define_textures.py                     16      0   100%
rayflare/textures/standard_rt_textures.py                58     12    79%
rayflare/transfer_matrix_method/__init__.py               1      0   100%
rayflare/transfer_matrix_method/lookup_table.py          74     61    18%
rayflare/transfer_matrix_method/tmm.py                  294    146    50%
rayflare/utilities.py                                    24      4    83%
-------------------------------------------------------------------------
TOTAL                                                  2316   1153    50%

@kandersolar
Copy link
Author

Speculation: perhaps it has something to do with multiprocessing and lines executed by worker processes not getting counted correctly. That could definitely make things behave differently run-to-run and between machines...

@phoebe-p
Copy link
Member

phoebe-p commented Aug 9, 2021

Yes, good thinking. It also makes sense that in that case files which are covered separately (e.g. analytic/diffraction.py and angles.py) are consistent between my local run and the codecov report while the actual optical calculation parts which can happen in parallel are not. I will just make a new branch locally and switch all the calculations to not in parallel - this will definitely change the coverage somewhat, because obviously then the parts of the code for parallel execution will not be tested, but should give a more similar overall number if that is the cause.

@phoebe-p
Copy link
Member

phoebe-p commented Aug 10, 2021

Ok, changing everything to run not in parallel improves the coverage somewhat (50% to 60%), but actually checking the coverage reports indicates that there is a more fundamental issue. For the rt.py, tmm.py and rcwa.py files, the coverage reports indicate that there is no coverage whatsoever of the RT, TMM and RCWA functions (i.e. the functions which generate matrices for the angular redistribution matrix method). This is incorrect, because these methods are used in several of the examples in test_compare_methods. The only thing I can think of is that codecov considers a part of the code covered if it is used while running the tests, and thus considers these functions (mostly) covered, while pytest imposes some more strict criterion, like that the function has to be called directly (which RCWA, TMM and RT are not -- they are called by process_structure in the test functions).

@kandersolar
Copy link
Author

I've made some partial progress here. I added a couple files in the repository root:

$ cat .coveragerc
[run]
parallel = True
source = rayflare
$ cat sitecustomize.py 
try:
    import coverage
    coverage.process_startup()
except ImportError:
    pass
  • The .coveragerc file is needed for the parallel=True option, which means each subprocess will generate its own coverage file with a unique filename instead of all subprocesses using the same filename and clobbering each others' results.
  • The sitecustomize.py file is needed so that coverage is enabled in the subprocesses in addition to the parent process.

Then I calculate coverage with this sequence:

  • Generate coverage files: PYTHONPATH=. COVERAGE_PROCESS_START=.coveragerc coverage run -m pytest
    • Setting the environment variables is very important: PYTHONPATH is needed for sitecustomize.py to be recognized and executed; COVERAGE_PROCESS_START is needed for coverage.process_startup() to do anything.
  • Merge coverage files: coverage combine
  • Summarize: coverage report
Name                                                  Stmts   Miss  Cover
-------------------------------------------------------------------------
rayflare/__init__.py                                      0      0   100%
rayflare/analytic/__init__.py                             0      0   100%
rayflare/analytic/diffraction.py                         63     22    65%
rayflare/angles.py                                       57     28    51%
rayflare/matrix_formalism/__init__.py                     2      0   100%
rayflare/matrix_formalism/ideal_cases.py                 48      4    92%
rayflare/matrix_formalism/multiply_matrices.py          261      7    97%
rayflare/matrix_formalism/process_structure.py           97      4    96%
rayflare/options.py                                      31      0   100%
rayflare/ray_tracing/__init__.py                          1      0   100%
rayflare/ray_tracing/rt.py                              613    139    77%
rayflare/rigorous_coupled_wave_analysis/__init__.py       1      0   100%
rayflare/rigorous_coupled_wave_analysis/rcwa.py         621    258    58%
rayflare/state.py                                        12      3    75%
rayflare/structure.py                                    40      5    88%
rayflare/textures/__init__.py                             2      0   100%
rayflare/textures/define_textures.py                     16      0   100%
rayflare/textures/standard_rt_textures.py                58     12    79%
rayflare/transfer_matrix_method/__init__.py               1      0   100%
rayflare/transfer_matrix_method/lookup_table.py          74      9    88%
rayflare/transfer_matrix_method/tmm.py                  294    145    51%
rayflare/utilities.py                                    24      4    83%
-------------------------------------------------------------------------
TOTAL                                                  2316    640    72%

So there's progress, but still lower than what codecov is finding. Here's the full coverage report: htmlcov.zip. @phoebe-p maybe you can take a look at this report (or follow the above steps yourself) and see what you think? Hopefully it should be more accurate about what is actually getting run.

I think this is beyond the scope of the JOSS review at this point -- I'm happy to continue helping figure this out, but I don't think there's any need to hold up the review for it.

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

No branches or pull requests

2 participants