Animals - Aquatic Reproduction and External Migration

[TaranRallings]

Description

READY NOW

This PR handles both aquatic reproduction and external migration from the system. These are closely related problems, which is why I did them together.

The commonality is that both involve "pausing" a cohort with respect to the update sequence. When a cohort is born aquatically (frogs etc) or externally migrates, it is taken out of the active_cohorts pools and is placed in either the migrated_cohorts or aquatic_cohorts pools. They remain there for a (yet unparameterized) amount of time, either developing or doing migrationy stuff. On return, they experience mortality and are returned to the active_cohorts pool in the same grid square they originally left from.

External migration happens with any cohort that has the external migration trait. My assumption is that this will mostly be seasonal but I have left the seasonality logic undeveloped for now and will have some conversations about that shortly.

Aquatic migration is part of the birth process. An aquatically reproducing organism births a new cohort into stasis. I have coded this trait as direct development even though this is not scientifically correct. It could be expanded upon later.

As I was working on this, it became clear that my birth method was doing far too many things so I have also broken that up into a series of helper methods. The logic (and testing) should now be easier to follow. Blessedly, no big problems were discovered in the process.

A lot of files were changed but almost all of the real content is in animal_model.py.

Fixes # (issue)

Type of change

• New feature (non-breaking change which adds functionality)
• Optimization (back-end change that speeds up the code)
• Bug fix (non-breaking change which fixes an issue)

Key checklist

• Make sure you've run the pre-commit checks: $ pre-commit run -a
• All tests pass: $ poetry run pytest

Further checks

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works
• Relevant documentation reviewed and updated

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 96.37681% with 5 lines in your changes missing coverage. Please review.

Project coverage is 94.08%. Comparing base (e5cd021) to head (4ca9201).

Files with missing lines	Patch %	Lines
virtual_ecosystem/models/animal/animal_model.py	95.41%	5 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #775      +/-   ##
===========================================
- Coverage    94.65%   94.08%   -0.58%     
===========================================
  Files           74       74              
  Lines         5127     5220      +93     
===========================================
+ Hits          4853     4911      +58     
- Misses         274      309      +35     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[dalonsoa]

Just to confirm, should I review or wait, considering the message at the top?

[dalonsoa]

Scientifically speaking, I can't say, but code-wise this seems pretty neat! It would have been best to separate in different PRs the refactor of the birth method and the addition of the new functionality, but changes can be followed well, so no problem.

[alexdewar]

Looks good! Agree with @dalonsoa: the PR's maybe a bit on the long side, but the code is nice and readable, so it wasn't too hard to review.

I've added a few comments/suggestions. As I've said in the comments, I'd personally tweak things so that remaining_time_away and individuals can never be <0 (i.e. if they dip below this, explicitly set them to zero). Otherwise it makes their semantics a bit confusing and it's a potential future footgun.

[alexdewar]

So this function does have the consequence that remaining_time_away might be left as less than zero afterwards. Not sure if you want to just set it to zero in this case? It might be nice if other code can rely on it being >=0 (not sure if other code is actually using it though)

[alexdewar]

Are migration seasons a relevant concept if the migration type isn't seasonal? If not, I'd be inclined to make is_migration_season just return false in this case and document this behaviour in the docstring.