989 foraging methods refactor to reduce resuse#990
Conversation
…maining foraging tests that broke.
TaranRallings
requested review from
Baizurah,
alexdewar,
annarallings,
dalonsoa,
davidorme,
nickwctan and
sallymatson
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #990 +/- ##
===========================================
+ Coverage 94.31% 94.82% +0.51%
===========================================
Files 79 79
Lines 6576 6511 -65
===========================================
- Hits 6202 6174 -28
+ Misses 374 337 -37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dalonsoa
left a comment
dalonsoa
left a comment
There was a problem hiding this comment.
Just a couple of small comments, but otherwise it looks good, so approving.
| consumed = target.mass_current * (1.0 - exp(-F * adjusted_dt)) | ||
| return max(consumed, 0.0) |
There was a problem hiding this comment.
If F is positive (I assume adjusted_dt is positive), then consumed can not be negative, so the max(consumed, 0.0) bit is not really necessary:
| consumed = target.mass_current * (1.0 - exp(-F * adjusted_dt)) | |
| return max(consumed, 0.0) | |
| return target.mass_current * (1.0 - exp(-F * adjusted_dt)) |
| herbivory_waste_pools[plant.cell_id].add_waste(plant_litter_cnp) | ||
| if herbivory_waste_pools and litter_cnp: | ||
| if resource.cell_id not in herbivory_waste_pools: | ||
| raise KeyError(f"Missing waste pool for cell {resource.cell_id}") |
There was a problem hiding this comment.
Why or when could this happen?
There was a problem hiding this comment.
I am probably overdoing it here but in general this isn't me anticipating a specific problem as much as it is laying a groundwork to help the inevitable long road of debugging.
| return total_consumed_mass | ||
|
|
||
| def calculate_consumed_mass_herbivory( | ||
| def default_consumed_resource_mass( |
There was a problem hiding this comment.
I'm not sure if there's going to be ever a consumed resource mass other than this one, but if there is not and the only point here is to re-use this same code by other methods of the class, the usual approach is to just make it "private" and remove the default. Otherwise, it will be understood that there is an option for not being default:
| def default_consumed_resource_mass( | |
| def _consumed_resource_mass( |
There was a problem hiding this comment.
Ah, good to know, thanks!
TaranRallings
deleted the
989-foraging-methods-refactor-to-reduce-resuse
branch
3 participants
Description
This is a small refactor to prepare the foraging system for all of the new resource pools I am introducing. It collapses the many identical
delta_massandcalculate_consumed_massmethods into something more DRY.Predation is unchanged.
Non-predation consumption now keys off two core methods:
default_consumed_resource_massforage_resource_listSmall, resource-specific, methods that serve to call
forage_resource_listare implemented to aid in bug hunting and facilitate later expansions.Fixes # (issue)
Type of change
Key checklist
pre-commitchecks:$ pre-commit run -a$ poetry run pytestFurther checks