Feature/animal module - Energy to Mass Conversion #338

TaranRallings · 2023-10-23T14:52:42Z

Description

I am aiming to align the animal module with Madingley before the winter break. To that end, I have converted the animal module to run on wet biomass as they do and implemented some of their parametrizations.

This is a long PR but it is mostly small changes of language (replacing energy with mass), some flow re-routing, and updating the tests. One of the larger changes is to reproduction. The model now tracks core body mass and reproductive body mass as different pools and draws the material of reproductive events from the reproductive mass pool specifically. This reproductive system will be expanded when I implement semelparity.

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
[x 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

… mass spent on the event.

…sholding to trophic flow of mass to reproductive mass.

…rework of tests.

…plant resources.

alexdewar · 2023-10-25T09:19:51Z

@TaranRallings Is this ready for review now?

dalonsoa

I've left a couple of comments worth to look at, but otherwise it looks good!

virtual_rainforest/models/animals/scaling_functions.py

dalonsoa · 2023-10-27T12:23:29Z

virtual_rainforest/models/animals/scaling_functions.py

+    Es = 3.7 * 10 ** (-2)  # energy to mass conversion constant (g/kJ)
+    sig = 0.5  # proportion of time-step with temp in active range (toy)
+    Ea = 0.69  # aggregate activation energy of metabolic reactions


Does this conform with the decided approach of including constants? If not, the standardised approach should be used.

It does not! I am doing a full constants rework immediately after this PR to avoid the PR sprawling out into the whole animal module.

dalonsoa · 2023-10-27T12:41:13Z

virtual_rainforest/models/animals/animal_cohorts.py

+        try:
+            if self.functional_group.diet == DietType.HERBIVORE and plant_list:
+                food_choice = choice(plant_list)
+                mass_consumed = self.eat(food_choice, excrement_pool)
+            elif self.functional_group.diet == DietType.CARNIVORE and animal_list:
+                food_choice = choice(animal_list)
+                mass_consumed = self.eat(food_choice, carcass_pool)
+            else:
+                LOGGER.info("No food available.")
+                food_choice = None  # No food available
+
+        except ValueError as e:
+            if str(e) == "Individuals cannot be 0.":
+                LOGGER.warning("Tried to eat with zero individuals in the cohort.")
+                mass_consumed = 0
+                food_choice = None  # No food was actually consumed
+            else:
+                raise


I'm not convinced by this approach. Using the text of a exception to decide what to do seems a bit hacky to me.

My personal preference would be to check if individuals are zero in the appropriate function and then return the correct value for food_choice and mass_consumed. If that is deemed to complicated to implement, then create a custom exception, eg. EatingZeroIndividualsCohortError, catch that (and only that) to act accordingly, leaving any other exception unhandled.

Ah, I see what you mean! I'll get that worked out.

alexdewar

I finally got round to finishing the review I started weeks ago 😆.

I've made some minor suggestions, but nothing that should be a blocker.

alexdewar · 2023-10-24T09:20:54Z

virtual_rainforest/models/animals/animal_communities.py

+        number_offspring = int(
+            (parent_cohort.reproductive_mass * parent_cohort.individuals)
+            / parent_cohort.functional_group.birth_mass
+        )


I guess you're rounding down on purpose here?

Yes, there must be reproductive mass enough to create a full offspring.

alexdewar · 2023-10-24T09:25:45Z

virtual_rainforest/models/animals/plant_resources.py

-        self.mass: float = data["layer_leaf_mass"].sum(dim="layers").to_numpy()[cell_id]
+        self.mass_current: float = (
+            data["layer_leaf_mass"].sum(dim="layers").to_numpy()[cell_id]
+        )


It looks like you're calculating multiple sums then only using one of them. Could you just calculate the sum you need?

If I remember correctly, and I may not be remembering correctly, this is some of @davidorme 's work from the plant module implementation. If he has no objections to the change then I am happy with it.

alexdewar · 2023-11-14T10:48:32Z

virtual_rainforest/models/animals/animal_communities.py

+            if (
+                consumer_cohort.individuals == 0
+            ):  # temporary while finalizing die_cohort placements
+                continue


I suppose you could print a warning here for now, so you don't forget to remove it.

alexdewar · 2023-11-14T10:49:13Z

virtual_rainforest/models/animals/animal_communities.py

+                if (
+                    min_size <= cohort.mass_current <= max_size
+                    and cohort.individuals != 0
+                    and cohort != consumer_cohort


I think you want to compare the identity of these objects:

Suggested change

and cohort != consumer_cohort

and cohort is not consumer_cohort

alexdewar · 2023-11-14T10:51:14Z

virtual_rainforest/models/animals/constants.py

+            MetabolicType.ENDOTHERMIC: {
+                "basal": (4.19 * 10**10, 0.69),
+                "field": (9.08 * 10**11, 0.7),
+            },
+            MetabolicType.ECTOTHERMIC: {
+                "basal": (4.19 * 10**10, 0.69),
+                "field": (1.49 * 10**11, 0.88),
+            },


You can write scientific notation like this:

Suggested change

MetabolicType.ENDOTHERMIC: {

"basal": (4.19 * 10**10, 0.69),

"field": (9.08 * 10**11, 0.7),

},

MetabolicType.ECTOTHERMIC: {

"basal": (4.19 * 10**10, 0.69),

"field": (1.49 * 10**11, 0.88),

},

MetabolicType.ENDOTHERMIC: {

"basal": (4.19e10, 0.69),

"field": (9.08e11, 0.7),

},

MetabolicType.ECTOTHERMIC: {

"basal": (4.19e10, 0.69),

"field": (1.49e11, 0.88),

},

I am wondering whether these would be better as dataclasses rather than dicts.

I am doing a constants rework immediately after this PR to avoid the PR sprawling out into the whole animal module.

alexdewar · 2023-11-14T10:51:46Z

virtual_rainforest/models/animals/constants.py

+        "basal": (4.19 * 10**10, 0.69),
+        "field": (9.08 * 10**11, 0.7),


Suggested change

"basal": (4.19 * 10**10, 0.69),

"field": (9.08 * 10**11, 0.7),

"basal": (4.19e10, 0.69),

"field": (9.08e11, 0.7),

alexdewar · 2023-11-14T10:52:04Z

virtual_rainforest/models/animals/constants.py

+        "basal": (4.19 * 10**10, 0.69),
+        "field": (1.49 * 10**11, 0.88),


Suggested change

"basal": (4.19 * 10**10, 0.69),

"field": (1.49 * 10**11, 0.88),

"basal": (4.19e10, 0.69),

"field": (1.49e11, 0.88),

alexdewar · 2023-11-14T10:53:25Z

virtual_rainforest/models/animals/constants.py

+    1.0  # Toy value for threshold of trophic flow to reproductive mass.
+)
+
+DISPERSAL_MASS_THRESHOLD: float = 0.75  # Toy value for thesholding dispersal.


In general it's better to describe constants with docstrings cf. comments. That way sphinx will be able to pick them up (as will your IDE).

Suggested change

DISPERSAL_MASS_THRESHOLD: float = 0.75 # Toy value for thesholding dispersal.

DISPERSAL_MASS_THRESHOLD: float = 0.75

"""Toy value for thesholding dispersal."""

(repeating myself just so it's clear what I'm doing)
I'll be doing a full constants rework after this PR to bring everything in line, including docstrings.

…dling in cohort foraging

…pecific cell.

codecov-commenter · 2023-11-16T15:50:46Z

Codecov Report

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

Comparison is base (5b1a77d) 96.27% compared to head (f840129) 95.69%.

Files	Patch %	Lines
...irtual_rainforest/models/animals/animal_cohorts.py	81.39%	8 Missing ⚠️
...al_rainforest/models/animals/animal_communities.py	92.30%	1 Missing ⚠️
...ual_rainforest/models/animals/scaling_functions.py	94.44%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #338      +/-   ##
===========================================
- Coverage    96.27%   95.69%   -0.59%     
===========================================
  Files           51       51              
  Lines         2499     2534      +35     
===========================================
+ Hits          2406     2425      +19     
- Misses          93      109      +16

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

TaranRallings added 12 commits September 22, 2023 15:07

Switching to use of Data[air_temperature].

288531f

Merge branch 'develop' into feature/animal_vr_run_integration

3d8445b

Fixed small indexing bug.

3d176d2

Integrated temperature from data.

2f2e22c

Made number of individuals in a new cohort a function of reproductive…

ed722df

… mass spent on the event.

Changed cohort functions to use mass.

b1931be

Reworked metabolism to follow Madingley.

1845b52

Collapsed mass threshold functions into single method and adding thre…

9f3d8c0

…sholding to trophic flow of mass to reproductive mass.

A number of changes to convert from energy to mass with accompanying …

ae6b49e

…rework of tests.

Fixed error in dummy_plants testing.

3948374

Merge branch 'develop' into feature/animal_module

0a014a4

Updated testing data instance so that it include air temperature and …

2adae11

…plant resources.

TaranRallings requested review from dalonsoa, davidorme, alexdewar and robewers01 October 23, 2023 14:52

TaranRallings self-assigned this Oct 23, 2023

dalonsoa approved these changes Oct 27, 2023

View reviewed changes

alexdewar approved these changes Nov 14, 2023

View reviewed changes

TaranRallings added 2 commits November 15, 2023 15:29

Small modifications to address reviews. Exponental formats, error han…

1b8e8cc

…dling in cohort foraging

Changed plant resource mass determination to only calculate for the s…

f840129

…pecific cell.

TaranRallings merged commit 8a586fe into develop Nov 16, 2023
22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/animal module - Energy to Mass Conversion #338

Feature/animal module - Energy to Mass Conversion #338

TaranRallings commented Oct 23, 2023 •

edited

Loading

alexdewar commented Oct 25, 2023

dalonsoa left a comment

dalonsoa Oct 27, 2023

TaranRallings Nov 15, 2023

dalonsoa Oct 27, 2023

TaranRallings Nov 15, 2023

alexdewar left a comment

alexdewar Oct 24, 2023

TaranRallings Nov 15, 2023 •

edited

Loading

alexdewar Oct 24, 2023

TaranRallings Nov 15, 2023

alexdewar Nov 14, 2023

alexdewar Nov 14, 2023

alexdewar Nov 14, 2023

TaranRallings Nov 15, 2023

alexdewar Nov 14, 2023

alexdewar Nov 14, 2023

alexdewar Nov 14, 2023

TaranRallings Nov 15, 2023

codecov-commenter commented Nov 16, 2023

	and cohort != consumer_cohort
	and cohort is not consumer_cohort

		"basal": (4.19 * 10**10, 0.69),
		"field": (9.08 * 10**11, 0.7),

		"basal": (4.19 * 10**10, 0.69),
		"field": (1.49 * 10**11, 0.88),

	DISPERSAL_MASS_THRESHOLD: float = 0.75 # Toy value for thesholding dispersal.
	DISPERSAL_MASS_THRESHOLD: float = 0.75
	"""Toy value for thesholding dispersal."""

Feature/animal module - Energy to Mass Conversion #338

Feature/animal module - Energy to Mass Conversion #338

Conversation

TaranRallings commented Oct 23, 2023 • edited Loading

Description

Type of change

Key checklist

Further checks

alexdewar commented Oct 25, 2023

dalonsoa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexdewar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TaranRallings Nov 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Nov 16, 2023

Codecov Report

TaranRallings commented Oct 23, 2023 •

edited

Loading

TaranRallings Nov 15, 2023 •

edited

Loading