Use multihost API with single host

[petrasovaa]

Update to latest pops-core and use new API inside pops.cpp using multi-host with a single host.

[petrasovaa]

@ChrisJones687 This seems to be finally working, right now the pops_model_cpp has the same API, just uses the new pops-core API. Do you want to merge this first and continue in a separate PR with the actual implementation of multi-host that would modify the API, or continue here?

[ChrisJones687]

Yes, let's merge this and then open a new commit for the rest of this.

[petrasovaa]

Weird, looks like tests on mac os ended with some errors. Any idea?

[ChrisJones687]

That is weird. It has never failed those tests before. That makes it seem like something has changed. I will pull it and run it locally to see what is happening.

[codecov]

Codecov Report

Attention: 140 lines in your changes are missing coverage. Please review.

Comparison is base (bfb05a8) 84.87% compared to head (fb9604d) 84.32%.
Report is 2 commits behind head on main.

❗ Current head fb9604d differs from pull request most recent head cff1f13. Consider uploading reports for the commit cff1f13 to get more accurate results

Files	Patch %	Lines
inst/include/host_pool.hpp	84.61%	44 Missing ⚠️
inst/include/competency_table.hpp	27.02%	27 Missing ⚠️
inst/include/generator_provider.hpp	42.42%	19 Missing ⚠️
inst/include/multi_host_pool.hpp	84.54%	17 Missing ⚠️
inst/include/environment.hpp	74.46%	12 Missing ⚠️
inst/include/config.hpp	81.08%	7 Missing ⚠️
inst/include/spread_rate.hpp	88.09%	5 Missing ⚠️
inst/include/model_type.hpp	66.66%	3 Missing ⚠️
inst/include/actions.hpp	98.14%	2 Missing ⚠️
inst/include/network.hpp	33.33%	2 Missing ⚠️
... and 1 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #186      +/-   ##
==========================================
- Coverage   84.87%   84.32%   -0.55%     
==========================================
  Files          51       59       +8     
  Lines        5647     6043     +396     
==========================================
+ Hits         4793     5096     +303     
- Misses        854      947      +93     

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

[ChrisJones687]

This fails locally for me as well. I don't think this means that the changes are incorrect just that for some reason the way the random seeds are being used here has changed the results for the specific random seed being used for these tests. In hindsight, these aren't the greatest tests because they can fail if a specific random draw has establishment occur early. This suggests that what we did changes the order of the stochastic processes.

As a note, these tests pass for other random seeds I have tried.

[wenzeslaus]

I'm wondering if there was a change in how random numbers are generated on macOS. Linux and Windows test pass, so I would say the order of operations is the same, but the random numbers supplied are different. I need to investigate that more.

...these aren't the greatest tests because they can fail if a specific random draw has establishment occur early. ...As a note, these tests pass for other random seeds I have tried.

I think there are just different types of tests, so we expect them to pass or fail at different circumstances. We ended up with naming some tests in r.pops.spread as strict and lenient. The strict are meant to fail with a different seed, but the margins for the lenient ones were informed by hundreds of different seeds.

[ChrisJones687]

That is a good point that there has to be a change to how MacOS does random number generation since it passes elsewhere else and has historically worked with MacOS.

True on for the expectation of the tests as these are meant to test for explicit results for that seed and have always worked well before.

[wenzeslaus]

Looking just at the code and ignoring the failing tests, the code itself looks good except the little issue with indentation.

[petrasovaa]

Looks like the tests on Mac are fixed!

[petrasovaa]

@ChrisJones687 one more complication, the tests fail when the host uncertainty is used. Vashek added this check https://github.com/ncsu-landscape-dynamics/pops-core/blob/main/include/pops/multi_host_pool.hpp#L365
to avoid probabilities > 1, and since the randomly generated hosts go above total population, the tests fail.

[ChrisJones687]

I pushed a check that ensures that hosts are never > than total population but it still fails. It is always like 1.002 or 1.005 which suggests to me that it is a rounding issue in pops-core. Thoughts?