Demographic model recombination rate

[gregorgorjanc]

Here is implementation for #1225.

I get almost all tests passing on my end:

• had to change np.NINF to -np.inf (new numpy)
• TestRegisterQCDFE.test_register_qc seems to have deep recursion, but that is not due to my changes (I think)

============================================================================== FAILURES ==============================================================================
_________________________________________________________________ TestRegisterQCDFE.test_register_qc _________________________________________________________________
[gw2] darwin -- Python 3.12.7 /opt/homebrew/Caskroom/miniconda/base/envs/popsim/bin/python

self = <tests.test_dfes.TestRegisterQCDFE object at 0x1094932f0>

    def test_register_qc(self):
        dfe = self.make_dfe("test")
        dfe.register_qc(dfe)
>       assert dfe.qc_dfe == dfe

tests/test_dfes.py:720:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
<attrs generated eq stdpopsim.dfe.DFE>:11: in __eq__
    self.qc_dfe == other.qc_dfe
<attrs generated eq stdpopsim.dfe.DFE>:11: in __eq__
    self.qc_dfe == other.qc_dfe
E   RecursionError: maximum recursion depth exceeded
!!! Recursion detected (same locals & position)
====================================================================== short test summary info =======================================================================
FAILED tests/test_dfes.py::TestRegisterQCDFE::test_register_qc - RecursionError: maximum recursion depth exceeded
==================================================================== 1 failed, 56 passed in 1.02s ====================================================================

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.81%. Comparing base (0d91be4) to head (feee774).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1591   +/-   ##
=======================================
  Coverage   99.81%   99.81%           
=======================================
  Files         130      130           
  Lines        4354     4376   +22     
  Branches      597      603    +6     
=======================================
+ Hits         4346     4368   +22     
  Misses          3        3           
  Partials        5        5           

Flag	Coverage Δ
	?

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[petrelharp]

Sorry for not noticing this so far, @gregorgorjanc , and thanks for getting this in! Having a look now.

[petrelharp]

Dang, this is a big one, @gregorgorjanc! I had a careful read through and only saw a few very minor fixes.

Also of note here - clarifying terminology around "genetic map" and "recombination map"; where now it's said that "genetic map" = crossovers, while "recombination map" = crossovers and optionally GC events also.

[petrelharp]

Looks like the only thing that isn't covered by tests is in the CLI? So, just needs something to tests/test_cli.py? Hm, I see you have got something in tests/test_cli.py, and I don't see why that isn't doing it.

[petrelharp]

So, this looks good to go, once that CLI test thing is figured out - are you able to do that, @gregorgorjanc?

[gregorgorjanc]

@petrelharp thanks for the review. I accepted all your suggested edits.

Will look into tests.

[petrelharp]

In figuring out how this works, I discovered we weren't doing what we wanted in this test.

[petrelharp]

We should be hitting that test for reals now!

[petrelharp]

Okay: pre-commit is failing, you updated the .pre-commit-config.yaml', and the new version of black` now requires a newer version of python. Updating the pre-commit versions is not generally something we should do in a PR because then you run the danger of changing a bunch of unrelated stuff (so the PR is messy). However:

• I can't locally run pre-commit run -a with the config file on upstream/main because it errors for some other obscure reason (maybe this is why you changed the config?)
• the changes it's making with the new versions of black, etc are not bad (AFAICT almost just one extra line after the header in all the files)

So, I'm going to keep the changes to pre-commit; merge this; and deal with pre-commit and the failing macOS CI in a different PR.

[gregorgorjanc]

@petrelharp thanks for the help here - yeah, I had to tinker with pre-commit as part of testing ..., which made a mess. Sorry. Thanks for the review and merging this in!