Nutrient intake #113

evadelmas · 2023-02-20T04:34:10Z

✨ New feature: Nutrient intake model

This PR includes some changes in the code architecture:

new producer_growth field in ModelParameters: this field contains the model for producer growth and the corresponding parameters. For now it can be one of either LogisticGrowth (with parameters K and α) or NutrientIntake (with all corresponding parameters)
argument K (carrying capacity) moved from environment to producer_growth
field producer_competition removed, α moved to producer_growth

evadelmas · 2023-02-20T04:36:23Z

.gitignore

src/model/dbdt.jl

src/model/model_parameters.jl

src/model/simulate.jl

evadelmas · 2023-02-20T18:33:11Z

I just realized that the extinction callback needs to be adjusted to avoid monitoring nutrients (added to the todo list above).

iago-lito

Hi @evadelmas and thank you for the PR :)

First off, there has been a small merge/rebasing glitch, but I think it's fixed now that your branch has been relocated to f14606c. One file seemed not to have benefited from the update: model_parameters.jl. The consequence is that producer_competition and temperature_dependence members of ModelParameters have been lost. But it's rather easy just to restore them. I have pointed the lines to restore in my comments below :)

The comments below are numerous, but don't trust the number: most of them are only small style edit suggestions. Also, some are repeated on every occurence of the same problem just to make sure we don't forget one. Also again, I have written them on-the-fly as I read, so a few early ones may be invalidated by later ones.

Some raise deeper topics, which I stress now in this summary:

The feature you are implementing here constitutes a strong extension to the semantics of the variables passed to solve(..), because B does not contain only biomasses now, but both biomasses and nutrients. As consequences, I think that:
- B needs to be renamed to something more generic. But we can delay that until the upcoming big renaming pass (Refresh identifiers names. #114).
- B[5:8] or solution[:, 5:8] becomes rather unsafe to do for users because we are not featuring some direct index ↦ species mapping anymore. I suggest that helper functions be written then like get_species(solution, 5:8) and get_nutrients(solution, 2:3), to dissociate species/nutrients indexes from simulation variables indexes. (see former discussion on this topic there with @alaindanet)
If I understand correctly. "Nutrient dynamics", as a feature, encompasses both "producers competition" and former "logistic growth".
- Regarding "producers competition", we need to decide what to do with @alaindanet's former files, especially producers_competition.jl. FWIU this PR copies some of his code into producer_growth.jl, but the original copy remains there and I think it's unused now. Also the parameter α is now duplicated within ModelParameters. I am not confident that its eventual location should be either ModelParameters.producers_competition.α or ModelParameters.producer_growth.α, because I think α will soon have "friends" and that they should all live in MultiplexNetwork. See Edges between species actually carry *data*. #82 about that. As a consequence, I think it's not a big deal where exactly α lives in the context of this PR, but it should be only one place, and we should not rely on it being there for long.
- Regarding "logistic growth", we need to decide what to do with the file productivity.jl, because I think it's now unused in the context of this PR. I see no problem with it being removed entirely, but we can't exactly do this yet because it still contains some boost code. I was not happy with the boost code being scattered throughout the project anyway, and I was planning on gathering it all into only a few files in the future. So maybe you can only delete non-boost-related functions within productivity.jl in the meantime.

And this is all about the deep issues to address :) I otherwise think the overall design of this PR is good, with a clear specification what the new producer_growth field does and a straightforward implementation of the nutrient dynamics. As you wrote, a few docstrings are missing yet. And the tests also do not pass. I believe it's a big step to get into updating them to feature nutrient dynamics, so keep up with the good work then :)

.vscode/settings.json

src/model/dbdt.jl

iago-lito · 2023-02-21T09:34:47Z

src/inputs/producer_growth.jl

+"""
+    LogisticGrowth(B, i, j)
+"""
+function (F::LogisticGrowth)(i, B, r, network::MultiplexNetwork, N = nothing)


Oh, unless we need to disambiguate two corner cases that I have been (perhaps erroneously) identifiying so far: if a model uses logistic growth, then there are no nutrients variables. Okay. But would it make sense that a model uses nutrients intake with no nutrient variables? In other terms, would there be any use in doing:

NutrientIntake(foodweb; n=0)

, which would still be different from LogisticGrowth(foodweb), or would it make no sense?

I used nothing because there are no nutrients in the logistic model, it's just here because I needed the two functions (LogisticGrowth and NutrientIntake) to have the same arguments.

Oh, interesting.. is it because you are using them later with some sort of macro or meta-programming approach?

iago-lito · 2023-02-21T09:37:26Z

src/inputs/producer_growth.jl

+function logisticgrowth(B, r, K, s)
+    !isnothing(K) || return 0
+    r * B * (1 - s / K)
+end


Okay, so IIUC these functions predate the content of file src/model/productivity.jl, righ? It would make sense that this file be deleted in the context of this PR then.. but now productivity.jl also contains boost-related code. I was intending to move that code elsewhere though. So I guess I can handle that when refactoring the boost later?

I think it's safe to remove productivity.jl - will do it in the next session

src/inputs/producer_growth.jl

iago-lito · 2023-02-21T09:45:32Z

src/inputs/producer_growth.jl

+    nutrientintake(B[i], r[i], F.Kₗᵢ[i,:], N) 
+end
+function nutrientintake(B, r, K, N)
+    !all(isnothing.(K)) || return 0


all(isnothing.(K)) && return 0

iago-lito · 2023-02-21T09:46:25Z

src/inputs/producer_growth.jl

+function nutrientintake(B, r, K, N)
+    !all(isnothing.(K)) || return 0
+    G_li = N ./ (K .+ N)
+    minG = minimum(G_li)


Oh, that's an interesting logic :) I guess you'll cite the paper in the docstring once you write it?

iago-lito · 2023-02-21T10:18:03Z

Wops, sorry @evadelmas I did not realize that you issued comments here before I finished my review. I'm addressing them rn :)

alaindanet · 2023-02-24T09:34:59Z

src/inputs/producer_growth.jl

+    D::Real = 0.25,
+    Sₗ::Union{Vector{<:Float64},<:Real} = repeat([10.0], n),
+    Cₗᵢ::Union{SparseMatrixCSC{<:Float64}, Vector{<:Float64}, Matrix{<:Float64}} = [range(1, 0.5, length = n);], 
+    Kₗᵢ::Union{Matrix{Union{Nothing,<:Real}}, <:Real} = 1.0)


May be we should find a less ambiguous name for half saturation rates $K_{ii}$, which is very similar to carrying capacity $K_i$ ?

I saw alternative notation like $Km_{ii}$ ($m$ for Michaelis-Menten) or $Ks_{ii}$ ($s$ for saturation). Another alternative could to have smallcase K, $k_{ii}$ as now we have $C_{ii}$ for nutrient and $c$ for consumer interference (may be still too ambiguous).

Good catch here @alaindanet! Maybe it's a discussion worth moving to #114? I think an effort to harmonize all names at once, and in particular single-letter names, will be very helpful to not eventually get drowned under short notations ^ ^"

Kickoff starter discussion #98.

Regarding this particular $K$ here, I suggest we don't have this discussion yet within the context of this PR, and leave the name as-is for now so as to not make the PR harder to land. Would you both be okay to delay the renaming until #114 is properly addressed?

…calNetworksDynamics.jl into nutrient_intake

iago-lito · 2023-05-02T08:04:58Z

This feature continues in #124.

iago-lito · 2023-06-27T12:55:51Z

Closed by #124.

evadelmas added 4 commits February 12, 2023 17:40

🚧 New method for producer growth

bd587b7

🚧 Functors for producer growth

5a0b0a9

remove vscode folder

209ed14

NIntake model's first draft

9acc96d

iago-lito self-requested a review February 20, 2023 13:22

iago-lito assigned evadelmas Feb 20, 2023

resolve conflicts

f14606c

iago-lito force-pushed the nutrient_intake branch from c7b1b6d to f14606c Compare February 20, 2023 13:44

iago-lito reviewed Feb 20, 2023

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

evadelmas commented Feb 20, 2023

View reviewed changes

src/model/dbdt.jl Outdated Show resolved Hide resolved

evadelmas commented Feb 20, 2023

View reviewed changes

src/model/model_parameters.jl Show resolved Hide resolved

evadelmas commented Feb 20, 2023

View reviewed changes

src/model/simulate.jl Outdated Show resolved Hide resolved

iago-lito requested changes Feb 21, 2023

View reviewed changes

This was referenced Feb 21, 2023

Refresh identifiers names. #114

Closed

Extend extinct_species() #110

Open

ModelParameters needs proper encapsulation. #84

Closed

alaindanet reviewed Feb 24, 2023

View reviewed changes

iago-lito force-pushed the dev branch from deb841b to aea091f Compare February 24, 2023 13:38

evadelmas added 9 commits April 22, 2023 13:43

🚧 New method for producer growth

f35cce0

🚧 Functors for producer growth

eca23b4

remove vscode folder

3f4f222

NIntake model's first draft

ccf132e

resolve conflicts

c0c1ee7

Merge branch 'nutrient_intake' of https://github.com/BecksLab/Ecologi…

dbb7740

…calNetworksDynamics.jl into nutrient_intake

clean

01a1767

Remove unecessary file

34cb056

change dndt to nutrient_dynamics for clarity

2322517

evadelmas added 7 commits April 23, 2023 21:32

Remove K from environment

3523b9c

Re-add temperature response (removed by merged conflict?)

c5beeaa

Debug

7a54461

adapt extinction callback

96ca57d

fix exports

30c7d8c

adapt / add tets

ed6a245

Start documenting

7308f0d

iago-lito closed this Jun 27, 2023

iago-lito deleted the nutrient_intake branch March 12, 2024 09:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Nutrient intake #113

Nutrient intake #113

evadelmas commented Feb 20, 2023

evadelmas commented Feb 20, 2023 •

edited

Loading

evadelmas commented Feb 20, 2023 •

edited

Loading

iago-lito left a comment •

edited

Loading

iago-lito Feb 21, 2023

iago-lito Feb 23, 2023

evadelmas Apr 24, 2023

iago-lito Apr 24, 2023

iago-lito Feb 21, 2023

evadelmas Apr 24, 2023

iago-lito Feb 21, 2023

iago-lito Feb 21, 2023

iago-lito commented Feb 21, 2023

alaindanet Feb 24, 2023 •

edited

Loading

iago-lito Feb 24, 2023

iago-lito commented May 2, 2023

iago-lito commented Jun 27, 2023

Nutrient intake #113

Nutrient intake #113

Conversation

evadelmas commented Feb 20, 2023

evadelmas commented Feb 20, 2023 • edited Loading

evadelmas commented Feb 20, 2023 • edited Loading

iago-lito left a comment • 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

iago-lito commented Feb 21, 2023

alaindanet Feb 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iago-lito commented May 2, 2023

iago-lito commented Jun 27, 2023

evadelmas commented Feb 20, 2023 •

edited

Loading

evadelmas commented Feb 20, 2023 •

edited

Loading

iago-lito left a comment •

edited

Loading

alaindanet Feb 24, 2023 •

edited

Loading