Nutrient intake continued.. #124
Conversation
There was a problem hiding this comment.
Hi @ismael-lajaaiti and thank you for pushing this feature forward :) Here is a wide list of nits, with nothing strongly emerging from the comments. The integration is well done, and you have taken this opportunity to keep cleaning up the project <3
I haven't had a deep look into the tests: in your opinion, are nutrient intake sufficiently tested yet?
There was a problem hiding this comment.
Good job again @ismael-lajaaiti :) I'll let you address these few remainers and get back to you regarding the boosting of nutrient dynamics and its connection to #82.
| @@ -140,9 +141,13 @@ export MultiplexNetwork | |||
| export n_links | |||
| export nestedhierarchymodel | |||
| export nichemodel | |||
| export NIntakeParams | |||
There was a problem hiding this comment.
? Does that still exist?
|
Oh, and regardless of the small requested changes, I think this PR is ready for "englishing" @Thomalpas. Can I leave this to your sharp eye? :) |
|
Of course! |
|
Thanks @iago-lito and @Thomalpas for your review and useful comments. Here are my corrections. |
| environment = Environment(foodweb; K) | ||
| params = ModelParameters(foodweb; functional_response, environment) | ||
| producer_growth = LogisticGrowth(foodweb; K) | ||
| params = ModelParameters(foodweb; functional_response) |
There was a problem hiding this comment.
Wait, the variable producer_growth is not used, and functional_response is not defined? Are they supposed to be the same name? Why do the tests pass then?
There was a problem hiding this comment.
functional_response is defined above in the file.
My bad for not using producer_growth, the output figure doesn't make sense, but I forgot to check it.
It should be fixed in the next commit.
Here is my take on Eva's PR 🚀🌔
I am sorry my first commit is a bit messy (not very 'atomic').
Here is a summary of my changes:
dBdt!todudt!NutrientIntakefields for sake of clarity.