Feature/stoichiometry for Animals

[TaranRallings]

Description

This PR contains the transition from a purely carbon mass system to a carbon, nitrogen, phosphorus (cnp) system.

Key Changes:

• the cnp mass dictionary is now the core mass used in most methods
• mass_current (total mass) is now a property, as is reproductive_mass, as the total values are sometimes needed
• a 'grow' method has been implemented in AnimalCohort to ensure stoichiometric ratios are adhered to
• a lot of tests were reworked and expanded.

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 92.13483% with 21 lines in your changes missing coverage. Please review.

Project coverage is 94.40%. Comparing base (39f2f88) to head (241c5a0).

Files with missing lines	Patch %	Lines
virtual_ecosystem/models/animal/animal_cohorts.py	92.20%	12 Missing ⚠️
virtual_ecosystem/models/animal/decay.py	91.89%	6 Missing ⚠️
virtual_ecosystem/models/animal/plant_resources.py	88.88%	2 Missing ⚠️
virtual_ecosystem/models/animal/animal_model.py	94.44%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #726      +/-   ##
===========================================
- Coverage    94.70%   94.40%   -0.30%     
===========================================
  Files           73       73              
  Lines         4946     5059     +113     
===========================================
+ Hits          4684     4776      +92     
- Misses         262      283      +21     

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

[jacobcook1995]

I've looked at the exchanges with the litter/soil model and I am happy that information will still get passed into my models in the format I want it in. I'm not approving as I don't feel well placed to give feedback on the overall structure/approach.

I had one specific comment which directly contradicts what I told you on teams earlier, sorry!

[jacobcook1995]

Sorry despite what I said earlier using 0.0 here doesn't really work, but obviously using inf for the divide by zero case would break the downstream calculations. Not 100% sure what the best approach is here, I guess this is a value that you should never expect, so maybe we just accept this as a weird edge case?

[jacobcook1995]

Okay actually think you want to add the following between the if and the else

elif self.leaf_waste_pools[cell_id].mass_cnp["carbon"] > 0 np.inf```

[jacobcook1995]

(the above might not be valid, but you get the idea)

This means that if there is a carbon flow with no associated nitrogen/phosphorus an infinite value gets returned (which then raises an error downstream). And this makes sense because biological matter always contains N and P.

But this handles the "there's no phosphorus because there's no organic matter at all" case gracefully

[jacobcook1995]

You actually take a step to avoid division errors here, is there a reason not to do this in the plant waste products case above?

[dalonsoa]

I have not gone into the detail of the code, but don't have much time today and wanted to give you some early feedback. This very much calls for a dedicated class in which to put together all the cnp related manipulations. Something like the following will also facilitate manipulation and type checks:

@dataclass 
class CNP:
    carbon: float
    nitrogen: float
    phosphorus: float

    @property
    def total(self) -> float:
        return self.carbon + self.nitrogen + self.phosphorus

    def __getitem__(self, key):
        """In case you want dictionary-style access."""
        return getattr(self, key)

Or, conversely, you can make it inherit from dict and add properties, as needed:

class CNP(dict):

    @property
    def carbon(self) -> float:
        return self["carbon"]

    @property
    def nitrogen(self) -> float:
        return self["nitrogen"]

    @property
    def phosphorus(self) -> float:
        return self["phosphorus"]

    @property
    def total(self) -> float:
        return sum(self.values())

Given that CNP is something that seems to have some entity as a group, I think it deserves to be treated as such in the code, also facilitating its documentation.

[TaranRallings]

This very much calls for a dedicated class in which to put together all the cnp related manipulations.

@dalonsoa This is a really good idea and I'm annoyed that I didn't think of it! I'll make the adjustments.