avoid animal and manure modules if no animals simulated

[JoeWaddell]

Context

After changes in #2721, no_animal_task returns the new error!

What

This PR skips the Feed, Animal, and Manure modules if no animals are simulated.

Why

Those modules shouldn't need to be run if no animals are present - barring future changes where manure processing of user configured manure (e.g. external sources) should be handled appropriately.

One big consideration

Should Feed Storage be "happening" even without animals? My understanding is that it would only be impacting/recorded when fed out, but this might be an oversight. If so: we just need to move the check in simulation_engine.py a little lower down in the method.

Test plan

Updated existing unit test to always assume animals are simulated, but we should likely add a new unit test to ensure methods are or aren't being called when animals aren't simulated.

Input Changes

Output Changes

• N/A

Filter

[github-actions]

Current Coverage: %

Mypy errors on no_animal_daily_simulation_fix branch: 1677
Mypy errors on dev branch: 1677
No difference in error counts

[github-actions]

🚨 Please update the changelog. This PR cannot be merged until changelog.md is updated.
🚨 Some tests have failed.

[github-actions]

Current Coverage: 99%

Mypy errors on no_animal_daily_simulation_fix branch: 1677
Mypy errors on dev branch: 1677
No difference in error counts

[ew3361zh]

@JoeWaddell We had a brief discussion about this in the dev-team and think this should either have further discussion as to whether a no-animals simulation should (and should be assumed) to not run the feed/manure modules. Alternatively, if that's the definite intention of this task, we thought the task should be renamed to something like crop-and-soil-only which is more explicit about what's happening.

[JoeWaddell]

@JoeWaddell We had a brief discussion about this in the dev-team and think this should either have further discussion as to whether a no-animals simulation should (and should be assumed) to not run the feed/manure modules. Alternatively, if that's the definite intention of this task, we thought the task should be renamed to something like crop-and-soil-only which is more explicit about what's happening.

After quickly consulting with Kristan, what I'll do is move the new if statement back down to only route around the manure parts specifically: the little block where it collects manure from HerdManager and shunts it to the ManureManager. That way we can still call it a no_animals run, as it will still run the Crop and Soil and Feed modules, and much of the Animal module.

[github-actions]

Current Coverage: 99%

Mypy errors on no_animal_daily_simulation_fix branch: 1677
Mypy errors on dev branch: 1677
No difference in error counts

[ew3361zh]

LGTM, I have a couple small suggestions which should be easy enough to implement if you agree.

[ew3361zh]

rather than calling on IM every day, I think it would be better to set this as an attribute in the SimulationEngine after self.im = InputManager().

e.g.

class SimulationEngine:
...
self.im = InputManager()
self.simulate_animals = self.im.get_data("config.simulate_animals")
...

and then update this line:

if self.im.get_data("config.simulate_animals"):

to

if self.simulate_animals:

It would probably also be worth then using this self.simulate_animals later on in the generate_daily_manure_applications() call which also gets this data from IM.

This is assuming we wouldn't want the ability to change this midway through a simulation.

[ew3361zh]

I still think we should revisit the name of this task if we're not running manure daily routines but will leave that to the SME team.

[allisterakun]

I do agree with both of Niko's comments. Otherwise LGTM

[JoeWaddell]

Great comments, updated the code accordingly! I'll let @KFosterReed and/or @elle-andreen chime in re: the name change. I don't have a problem either way, but I think "no animal" still makes sense since the Manure module, as it works right now, logically shouldn't function without animals being simulated (but the other modules can).

[KFosterReed]

Thanks team! I agree with Joe that leaving the task as no_animal makes sense for now since it is dependent on the simulate_animals flag. As Joe and I have been discussing, we need to put our heads together in the future and think more broadling about the independence of the modules and how to effectively simulate them on their own.

[github-actions]

Current Coverage: 99%

Mypy errors on no_animal_daily_simulation_fix branch: 1677
Mypy errors on dev branch: 1677
No difference in error counts

[elle-andreen]

Thanks team! I agree with Joe that leaving the task as no_animal makes sense for now since it is dependent on the simulate_animals flag. As Joe and I have been discussing, we need to put our heads together in the future and think more broadling about the independence of the modules and how to effectively simulate them on their own.

@JoeWaddell @KFosterReed I agree that 'no_animal' still makes sense in this context!