Fix a failing test on DL3 Tool

[chaimain]

A test on DL3 Tools when using IRF interpolation methods was failing as it required a list of IRFs which are not present as fixtures for the scope of the test session. These IRFs are created and tested in lstchain/high_level/tests/test_interpolate (through some "lengthy" code) for almost all combinations of IRF interpolations.
As such, it is not essential to have separate and similar tests on DL3 Tools, for testing the same functionality.

This was missed in the earlier PR #711 as I tested in sequential order instead of in parallel (as done in CI).

In case, we require to have the DL3 Tools tested for the full functionality again, we would just need to shift ~150 lines of code from lstchain/high_level/tests/test_interpolate.py to lstchain/conftest.py

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: +1.14 🎉

Comparison is base (e41d81d) 72.77% compared to head (4564a0e) 73.92%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1092      +/-   ##
==========================================
+ Coverage   72.77%   73.92%   +1.14%     
==========================================
  Files         122      123       +1     
  Lines       11849    11861      +12     
==========================================
+ Hits         8623     8768     +145     
+ Misses       3226     3093     -133     

Impacted Files	Coverage Δ
lstchain/high_level/interpolate.py	93.42% <100.00%> (ø)
lstchain/high_level/tests/test_interpolate.py	100.00% <100.00%> (ø)
lstchain/tools/tests/test_tools.py	100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[moralejo]

Hi,
probably unrelated, but I realized that the tests in high_level do not pass in my Mac (while they work on linux).
This is the same for the master and for the interp_irfs branch

lstchain/data/tests/test_normalised_pulse_template.py::test_load_from_file_and_miscellaneous PASSED [  4%]
lstchain/high_level/tests/test_hdus.py::test_create_event_list ERROR     [  5%]
lstchain/high_level/tests/test_hdus.py::test_create_obs_hdu_index ERROR  [  5%]
lstchain/high_level/tests/test_hdus.py::test_get_timing_params PASSED    [  6%]
lstchain/high_level/tests/test_interpolate.py::test_load_irf_grid PASSED [  6%]
lstchain/high_level/tests/test_interpolate.py::test_interp_irf FAILED    [  7%]
lstchain/high_level/tests/test_interpolate.py::test_compare_irfs PASSED  [  7%]

When looking in more detail, I don't understand anything, the error seems to be in the r0 to dl1 step

==================================== ERRORS ====================================
___________________ ERROR at setup of test_create_event_list ___________________

temp_dir_observed_files = PosixPath('/private/var/folders/cn/jhp87yg56455zyhkzz36mhl40000gn/T/pytest-of-moralejo/pytest-18/observed_files0')
run_summary_path = PosixPath('/private/var/folders/cn/jhp87yg56455zyhkzz36mhl40000gn/T/pytest-of-moralejo/pytest-18/observed_files0/RunSummary_20200218.ecsv')

    @pytest.mark.private_data
    @pytest.fixture(scope="session")
    def observed_dl1_files(temp_dir_observed_files, run_summary_path):
...
...
E             File "/Users/moralejo/opt/miniconda3/envs/lst-dev/lib/python3.8/si
te-packages/astropy/io/ascii/core.py", line 743, in check_column_names
E               raise InconsistentTableError(
E           astropy.io.ascii.core.InconsistentTableError: Length of names argume
nt (10) does not match number of table columns (3)

Anyone has an idea of what may be going on?

[gabemery]

Ok with removing the test if it leads to failure. (also for some reason it doesn't reduce coverage?)

[gabemery]

@moralejo Is this the full error message you get? It doesn't include the line where you get the error.

[chaimain]

Ok with removing the test if it leads to failure. (also for some reason it doesn't reduce coverage?)

It does reduce the coverage, but of the DL3 Tool file, https://app.codecov.io/gh/cta-observatory/cta-lstchain/pull/1092/blob/lstchain/tools/lstchain_create_dl3_file.py which is an indirect change with respect to this PR.

[rlopezcoto]

before removing the test because it is not passing, let's try to figure out what is the problem.

[chaimain]

before removing the test because it is not passing, let's try to figure out what is the problem.

To perform the tests on the DL3 Tool using IRF interpolation methods, we need at least 3 sets of IRFs with different (zenith, delta) values for either or all types of IRFs:

• source-independent, source-dependent
• global cuts, energy-dependent cuts
• point-like, full enclosure

For the test MC, we just have a file for a single node (zenith, delta), and so, in the lstchain/high_level/tests/test_interpolate I create "fake" IRFs for 2 extra nodes using as a reference the simulated_irf_file (source-independent and with global cut) and a separate IRF that I create in that test en_dep_cut_irf_1.fits.gz (source-independent and with energy-dependent cuts). All these IRFs are stored in the same place.

In the test for DL3 Tool, I was assuming the presence of these extra IRFs to be used for each test process. Although, when performing sequential tests, this assumption holds true every time but in CI, where we have parallel tests being performed, this assumption fails sometimes.

The issue of test coverage is definitely there, but seeing the numerous options that can be tested, I felt it was not a strict requirement to cover ALL IRF combinations to test whether the IRF interpolation method works or not. The core interpolation methods are already tested and cleared in lstchain/high_level/tests/test_interpolate.

Hence, it was simpler to remove these tests on DL3 Tool for IRF interpolation methods.

To complete the full test coverage of the DL3 Tool, we would require to create all such numerous IRFs in lstchain/conftest.py (fixture for scope="session") and then any parallel pytest ordering will work properly.

To summarise, core IRF interpolation tests are passing in the high_level module, but for testing DL3 Tools in a parallel order (in CI), we need to have a LOT of IRFs created as fixtures.

[moralejo]

@moralejo Is this the full error message you get? It doesn't include the line where you get the error.

The error itself seems to happen when running lstchain_data_r0_to_dl1, and more specifically, in ctapipe_io_lst, when it tries to read in the drive report of the test data
https://github.com/cta-observatory/ctapipe_io_lst/blob/28b4a1b55e58fbacae6fe52c90b0e6e28433b984/ctapipe_io_lst/pointing.py#L96-L121

test_data/real/monitoring/DrivePositioning/drive_log_20200218.txt

astropy.io.ascii.core.InconsistentTableError: 
ERROR: Unable to guess table format with the guesses listed below:
[...]

And indeed, the format does not seem to be what it tries to read. I can reproduce it outside of the tests (both running the lstchain_data_r0_to_dl1 script and directly reading in the file)

astropy.io.ascii.core.InconsistentTableError: Length of names argument (10) does not match number of table columns (20)

By the way, I can reproduce that also in linux (the reading error I mean - due to a different, new issue, I cannot run the full tests) The tests work in linux (reproducing the table reading error means nothing, because that is not the proper reader for that file). Discussion continues in #1094

[gabemery]

@moralejo It seems like a dedicated issue will be needed to discuss this.

[moralejo]

@moralejo It seems like a dedicated issue will be needed to discuss this.

Let's discuss here: #1094

[rlopezcoto]

Now that we have worked out the problems with the tests in #1094 and that it was unrelated to this PR, @moralejo can you confirm that tests pass on this branch and that everything is alright for it to be merged?