tskit-dev / tskit

Population-scale genomics
MIT License
147 stars 70 forks source link

Add macros to LsHmm_forward_matrix and LsHmm_backward_matrix #2853

Closed szhan closed 9 months ago

szhan commented 9 months ago

Fixes #2841

codecov[bot] commented 9 months ago

Codecov Report

Merging #2853 (c5ccb42) into main (25116f6) will decrease coverage by 0.19%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2853      +/-   ##
==========================================
- Coverage   89.94%   89.76%   -0.19%     
==========================================
  Files          28       30       +2     
  Lines       22849    29907    +7058     
  Branches     4621     5803    +1182     
==========================================
+ Hits        20552    26845    +6293     
- Misses       1281     1755     +474     
- Partials     1016     1307     +291     
Flag Coverage Δ
c-tests 86.15% <ø> (ø)
lwt-tests 80.78% <ø> (ø)
python-c-tests 67.90% <100.00%> (?)
python-tests 98.92% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
python/_tskitmodule.c 88.69% <100.00%> (ø)

... and 4 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 25116f6...c5ccb42. Read the comment docs.

jeromekelleher commented 9 months ago

The linter is stupid here and doing the wrong thing. The only solution is to put in some comments to force it not to join lines that shouldn't be joined (see other usages of PyBEGIN etc)

szhan commented 9 months ago

lint is doing it in other places like

    Py_BEGIN_ALLOW_THREADS err = tsk_treeseq_genealogical_nearest_neighbours(
        self->tree_sequence, PyArray_DATA(focal_array), num_focal, reference_sets,
        reference_set_size, num_reference_sets, 0, PyArray_DATA(ret_array));
    Py_END_ALLOW_THREADS if (err != 0)
    {
        handle_library_error(err);
        goto out;
    }
szhan commented 9 months ago

I'm not sure how to test that the macros are working here. I guess we could add tests that look like the ones in test_threading.py, but I'm not sure if it is testing what we want in this case.

jeromekelleher commented 9 months ago

I think we should drop this and do your multi-threaded matching using processes instead @szhan