iago-lito
commented
1 year ago Wait at least on #87 and #113 to land first, so we avoid unnecessary churn/conflicts.
Closed iago-lito closed 8 months ago
iago-lito
commented
1 year ago Wait at least on #87 and #113 to land first, so we avoid unnecessary churn/conflicts.
iago-lito
commented
1 year ago Following #98, maybe we could also list and clarify all the single-lettered parameter names? And, if not renaming them, at least providing one explicit alias for each?
First in the list: α → attack_rate?
Second in the list: K → carrying_capacity, K → half_saturation_rate?
ismael-lajaaiti
commented
1 year ago Thank you for this list. I missed that issue, but it is really a great 👌 Here are my first suggestions:
ModelParameters such as BioRates or Environment. We could even drop the j functor and create classic functions function for the functional responses (e.g. classic_reponse(model::Model)).BioEnergeticFunctionalResponse should be removed.TemperatureResponse can be confusing with an alternative for the ClassicResponse, etc. Could we think of another word than "response"? @hanamayall ExponentialBA could become BoltzmannArrhenius? @hanamayall Allometry, but anyway you should refactor this part of the code. I really think we can do better there.get_full_matrix(interaction) function.EcologicalNetworks (e.g. nichemodel). IMO it would be too confusing for a user who would like to use both packages to change them.The duty calls me somewhere else, but I'll edit this message sooner or later with other suggestions. 🦸
iago-lito
commented
1 year ago Great, let's discuss that :D I'm updating the list at the beginning of this thread.
- I'm thinking with
ModelParametersneeds proper encapsulation. #84 that we could drop sub-structures inModelParameterssuch asBioRatesorEnvironment. We could even drop thejfunctor and create classic functions function for the functional responses (e.g.classic_reponse(model::Model)).
Agreed, but not before #84 happens.
BioEnergeticFunctionalResponseshould be removed.
Good :)
TemperatureResponsecan be confusing with an alternative for theClassicResponse, etc. Could we think of another word than "response"? @hanamayall
It could be confusing, but that is legitimate because "Response" is a rather vague term IMO. I don't see any significant semantic difference between "Response" and "Function" or "Functional Shape". As a consequence, I am not suprised that two different things end up being referred to as a "Response". And there is nothing to "FunctionalResponse" that is specific to the "Linear-/Bioenergetic-/Classical-" ones. My suggestion here would be to either embrace the vague-ness and expect users to not be confused, or to replace all "Response" terms with something more specific to either case:
Linear-/Bioenergetic-/Classical-Response? And how could it be replaced with something more specific to F?
Linear-/Bioenergetic-/Classical-Predation?TemperatureResponse? And how could it be replaced with something more specific to temperature?
ExponentialBAcould becomeBoltzmannArrhenius? @hanamayall
Yeah, could "<AnythingElse>BA" exist @hanamayall? Or is the "Boltzmann-Arrhenius" relation only "exponential"?
- I like
Allometry, but anyway you should refactor this part of the code. I really think we can do better there.
Sure, but I would say that the naming problem is orthogonal. If we can solve it first, let's do it before #74 even happens ;)
- For me "dense" and "full" have slightly different meaning, but I might be wrong, a "dense" interaction matrix has many non-zero coefficients (where interactions are feasible) but a "full" interaction matrix has only non-zero coeffiecients (where interactions are feasible). But I like the idea of a
get_full_matrix(interaction)function.
Oh, I see. So "dense" would refer to how the matrix is stored in memory, while "full" would refer to the value the matrix actually contains. Then.. I guess A_<nti>_full matrices are all "dense", but none is "full"?
- I'm against changing structural model name from
EcologicalNetworks(e.g.nichemodel). IMO it would be too confusing for a user who would like to use both packages to change them.
Yeah, that would definitely be confusing.
- macros shoudn't be exported
Good :)
hanamayall
commented
1 year ago Hi! I've named this implementation of temperature effects 'ExponentialBA' because there is also an extended Boltzmann-Arrhenius model, describing a unimodal relationship in which rates decrease at higher temperatures. This is one of the few alternative temperature responses that it would be good to integrate into the temperature dependence functionality in the future.
Regarding "Response", in my opinion the word is vague and common enough that hopefully Functional Response and Temperature Response are not going to be regularly confused by users.
iago-lito
commented
1 year ago Okay, so we can expect having both ExponentialBoltzmanArrhenius and ExtendedBoltzmannArrhenius one day in the future :)
Regarding "Response", in my opinion the word is vague and common enough that hopefully Functional Response and Temperature Response are not going to be regularly confused by users.
I also think it's okay to assume that there won't be much confusion, and that we can leave them as-is because they seem to be usual names. If we choose to keep that though, maybe we can just emit a useful error in case user ever mistakes ? Something along:
Error: Expected a FunctionalResponse (LinearResponse, ClassicResponse or BioenergeticResponse), received a TemperatureResponse instead (ExponentialBolztmannArrhenius).?
andbeck
commented
1 year ago We can’t change FunctionalReaponse. Despite how silly it is, it is embedded history and dogma.
However, the temperature stuff is not a response. It’s a scaling that allows us to modify rates and investigate a response to temperature.
So one idea is to rename that. TemperatureScaling (with options) and NoTemperatureScaling.
ismael-lajaaiti
commented
1 year ago We can’t change FunctionalReaponse. Despite how silly it is, it is embedded history and dogma.
However, the temperature stuff is not a response. It’s a scaling that allows us to modify rates and investigate a response to temperature.
So one idea is to rename that. TemperatureScaling (with options) and NoTemperatureScaling.
I really like this suggestion, this is what I had in mind but couldn't find the right word ('scaling').
ismael-lajaaiti
commented
1 year ago Here are my answers @iago-lito:
- I like
Allometry, but anyway you should refactor this part of the code. I really think we can do better there.Sure, but I would say that the naming problem is orthogonal. If we can solve it first, let's do it before #74 even happens ;)
Yes, you're right. So let's go for Allometry.
- For me "dense" and "full" have slightly different meaning, but I might be wrong, a "dense" interaction matrix has many non-zero coefficients (where interactions are feasible) but a "full" interaction matrix has only non-zero coeffiecients (where interactions are feasible). But I like the idea of a
get_full_matrix(interaction)function.Oh, I see. So "dense" would refer to how the matrix is stored in memory, while "full" would refer to the value the matrix actually contains. Then.. I guess
A_<nti>_fullmatrices are all "dense", but none is "full"?That's not what I had in mind. I meant that full implies dense, but not the reverse. A dense interaction matrix would be, as I understand it, a matrix with many interactions, but all possible interactions are not necessarily realized (i.e. dense but not necessarily full). By contrary, a full matrix would be matrix where all possible interactions are realized, and therefore with many interactions (i.e. full and therefore also dense). To go on this. I'm thinking that one possibility could be to rename (and change the function signature) such that
A_<nti>_full(net::EcologicalNetwork)becomesfill(net::EcologicalNetwork, interaction::Symbol)it would be more julian as we would write a new method for theBase.fillfunction.
I now go with points I didn't address before. Here are my suggestions:
allometric_rate -> get_rates the 'allometriness' is already in the mandatory argument of type AllometricParamsset_temperature! -> scale_with_temperature! / set_temperature_scaling!? A bit more explicit but also longer.get_extinct_species -> get_extinctions / get_extinction_events? As this function returns the species extinct as well as their time of extinction, we could create a new function get_extinct_species that would return the keys of the output of the future get_extinctions see #110 (and I was wondering @alaindanet are you working on that with your current PR? Or do you need my contribution on that?).species_persistence -> persistence for sake of brevity species_richness -> richness for consistency, if I remember well @alaindanet is already on that.iago-lito
commented
1 year ago Great, thank you for feedback @andbeck @ismael-lajaaiti @hanamayall :) (Any suggestion I don't discuss below is already integrated in the list.)
@andbeck We can’t change FunctionalReaponse. Despite how silly it is, it is embedded history and dogma.
Haha, I had a feeling that this would be true. Better not to confuse anyone with fancy dogma shaking then. Let's keep it as-is and work around it :) Keep the fancyness for the new names we introduce.
@andbeck However, the temperature stuff is not a response. It’s a scaling that allows us to modify rates and investigate a response to temperature. So one idea is to rename that. TemperatureScaling (with options) and NoTemperatureScaling.
@ismael-lajaaiti
set_temperature!->scale_with_temperature!/set_temperature_scaling!? A bit more explicit but also longer.
I very much like scale_with_temperature! because it reads like natural language, but this would be at odds with the others set_*! methods we are planning to list (#84), so maybe drop it :'(
I am also tempted to simplify these into TemperatureScale/NoTemperatureScale/ExponentialBolztmannArrheniusScale/set_temperature_scale!, but I am not 100% confident that this would not change the meaning in a slight-yet-wrong English way. Would set_temperature_scale!(model) and set_temperature_scaling!(model) mean the same to you @hanamayall @andbeck?
@ismael-lajaaiti
allometric_rate->get_ratesthe 'allometriness' is already in the mandatory argument of typeAllometricParams
I see. But would you not be tempted to use get_rates for anything then? Are there other (non-allometric) parameters that users could refer to as "rates"? If not, then good. If yes, but then we could add dedicated alternate methods like get_rates(model, :growth), then good as well. If this would be too confusing however, we would need to fall back on get_allometric_rates(model, allometry), which is kind of redundant indeed.
I meant that full implies dense, but not the reverse. A dense interaction matrix would be, as I understand it, a matrix with many interactions, but all possible interactions are not necessarily realized (i.e. dense but not necessarily full). By contrary, a full matrix would be matrix where all possible interactions are realized, and therefore with many interactions (i.e. full and therefore also dense).
If "dense" means "non-sparse" and refers to the way the matrix is implemented/stored, then I disagree that "full implies dense". Indeed: a sparse matrix could very well be full as well (although probably inefficient to use). See for instance:
julia> SparseMatrixCSC(ones(4, 4))
4×4 SparseMatrixCSC{Float64, Int64} with 16 stored entries:
1.0 1.0 1.0 1.0
1.0 1.0 1.0 1.0
1.0 1.0 1.0 1.0
1.0 1.0 1.0 1.0So I would say that the concepts of dense and full remain orthogonal:
| non-full (1) | full (2) | |
|---|---|---|
| dense (3) | a = [0 0; 0 1] ✓ |
b = ones(2, 2) ✓ |
| sparse (4) | c = SparseMatrixCSC([0 0; 0 1]) ✓ |
d = SparseMatrixCSC(ones(2, 2)) ✓ |
So, regardless of whether we agree on the meaning of the words full, sparse and dense, it is a fact that a, b, c and d are all possible values in Julia, so the (unnamed) columns (1) and (2) do exist, and the (unnamed) rows (3) and (4) do exist as well.
With this set up: what do you expect the values returned by the functions yet-named A_<nti>_full() to look like? a, b, c, d?
To go on this. I'm thinking that one possibility could be to rename (and change the function signature) such that
A_<nti>_full(net::EcologicalNetwork)becomesfill(net::EcologicalNetwork, interaction::Symbol)it would be more julian as we would write a new method for theBase.fillfunction.
Watch out: from the documentation of Base.fill, "julian" people would very much expect the following behaviour:
julia> fill(network, :refuge)
2×2 Matrix{Symbol}:
:refuge :refuge
:refuge :refuge.. which I suspect is not at all what you had in mind, right? ;)
alaindanet
commented
1 year ago * `get_extinct_species` -> `get_extinctions` / `get_extinction_events`? As this function returns the species extinct as well as their time of extinction, we could create a new function `get_extinct_species` that would return the keys of the output of the future `get_extinctions` see [Extend `extinct_species()` #110](https://github.com/BecksLab/EcologicalNetworksDynamics.jl/issues/110) (and I was wondering @alaindanet are you working on that with your current PR? Or do you need my contribution on that?). * `species_persistence` -> `persistence` for sake of brevity * `species_richness` -> `richness` for consistency, if I remember well @alaindanet is already on that.
@ismael-lajaaiti. You are right, I am on it for species_persistence and richness. Just that we decided some time ago that species_persistence would be nice for the sake of clarity.
For extinction events, I would be keen to treat it in a separate PR if it makes sense. I would definitely need your input on this :pray:
alaindanet
commented
1 year ago ModelParameters -> Model is an enumerate type with variants: Bioenergetic(...), LotkaVolterra(...)get_modelNetwork: always a MultiplexNetwork with one foodweb layer and optional 4 non trophic layers.(B/dB/dBdt!) → (u/du/dUdt!) (not exported)generate_dbdt → generate_dudtBioRates / Environment ->
x: metabolism,
y: metabolism_consumption,
d: mortality,
r: growth,
e: efficiency,
K: Carrying capacity,
alpha: competition,
\omega: consumer_preference,
Z: ppmr
M: massfoodweb_generator()FunctionalResponse:
LinearResponse: LinearClassicResponse: Unscaled:a_r: attack_rateh_t -> t_h: handling_timeBioenergeticResponse: ScaledB0: half_saturation_densityh: hill_exponentc -> i_intra: intra_specific_interferenceRemove BioEnergeticFunctionalResponse ⚠ Not defined within the project!
T -> Temperature
Scale to scaling:
TemperatureResponse → TemperatureScalingNoTemperatureResponse → NoTemperatureScalingExponentialBA → ExponentialBoltzmannArrheniusScalingAllometry: (Iago said ok) AllometricParams → Allometry (see #74 for possible evolution) ExponentialBAParams → ExponentialBoltzmannArrheniusAllometry (see #74) allometric_rate → get_rates / get_allometric_rates ?
It has not been reviewed below!
get_extinct_species → get_extinctions | see #110
∅ → get_extinct_species |species_richness → species_richness | Shorten. # Good
species_persistence
nutrient_richness17.
A_competition_full | Does 'full' actually means 'dense' here?
A_facilitation_full | Maybe a get_dense_matrix(:competition)
A_interference_full | would be a simpler option?
A_refuge_full |
cascademodel | re-exported from `EcologicalNetworks.jl`,
nichemodel | not much we can do about them
nestedhierarchymodel | unless we are willing to alias them?
mpnmodel | (not a *very* good idea: confusion ahead)
cpad | Out of scope: keep but don't export.
fitin | This is my bad: sorry ^ ^"
@check_between |
@check_greater_than | Do these really need to be exported?
@check_in | They feel out-of-scope.
@check_lower_than | => Stop exporting them.
@check_size |
foodweb_cv |
foodweb_evenness |
foodweb_richness | About to change with #87 anyway.
foodweb_shannon |
foodweb_simpson |
attack_rate |
boltzmann |
coefficient_of_variation | Nothing much to say yet. Any thoughts?
connectance |
draw_asymmetric_links |
draw_symmetric_links |
efficiency |
ExtinctionCallback |
find_steady_state |
handling_time |
homogeneous_preference |
interaction_names |
Layer |multiplex_network_parameters_names | n_links | nontrophic_adjacency_matrix | NonTrophicIntensity | population_stability | potential_competition_links | potential_facilitation_links | potential_interference_links | potential_refuge_links | predators_of | preys_of | producer_growth | ProducerCompetition | producers | richness | simulate | total_biomass | trophic_levels |
Before release, all identifiers in the project (or at least all
exported identifiers) should be screened for consistency, clarity, confusability, simplicity, conciseness etc.. Let's start discussing this now, I'm slacking the whole list here just to start discussion:Also I can think of these few non-exported identifiers for now:
Any other idea?
Regarding the files under
src/we could also take this opportunity to sort'em out a bit. In particular, the distinction between the three folders "before simulation / during simulation / after simulation" (IIUC) needs to be made clearer:Also, the files under the
test/folder do not need to be all prefixed bytest-, or to match the files undersrcexactly, as long as they all live undertest/and all features are tested. So what about the following?Parameter names should also be checked, see #2 for example.
What do you think ? :)