-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce Topology
to check biomass graph disconnections.
#152
base: main
Are you sure you want to change the base?
Conversation
49e0fad
to
32e1e6d
Compare
a90065c
to
6154382
Compare
43e3443
to
cb82c5f
Compare
522e2c4
to
bbcd64e
Compare
e4875de
to
1245552
Compare
0f165a1
to
b942776
Compare
Topology
to check biomass graph disconnections.
So, this PR addresses #151 in a very deep way by creating the new The reason to put so much effort into Review welcome @alaindanet @ismael-lajaaiti ;) There is a lot of code so I won't mind if you don't thoroughly scan all of it of course. But if you want to help: please test the feature to check whether it's comfy and whether it corresponds to something that would at least solve #151 :) Entrypoint into the feature:
Feedback welcome! (here or on the channel ;) |
Wops! I can reproduce your error with a simple graph indeed: m = default_model(Foodweb([:a => [:b, :c], :b => :c]))
g = m.topology
starving_consumers(m, g) This is a bug in the graph visit implementation. I'm fixing it right now. Thank you for reporting.
BTW are you happy with the default |
Great, thank you for fixing it. 😉
Yes, I like it. In particular, because with this default it is easy to distinguish between species and nutrients. |
f16e4c9
to
dbdb43c
Compare
I have just tried the feature too, I have an error checking topology with a solution object:
|
I've run a few tests (see the end of this message) and the methods seem to work well. But here are a few remarks:
My testsusing CairoMakie
using DataFrames
using Distributions
using EcologicalNetworksDynamics
using Random
Random.seed!(1234) # For reproducibility.
foodweb = Foodweb(:niche, S=50, C=0.2)
m = default_model(foodweb)
g = topology(m)
g == m.topology # Sanity check. Expected to be true.
sol = simulate(m, rand(Uniform(0.1, 1), m.S), 1_000)
extinctions(sol)
# 49 => 58.8748
# 35 => 511.178
# 16 => 640.776
# Some test to ensure that everything is working as expected.
# Before the first extinction, the topology should be the same as the original one.
all([topology(m) == topology(sol, t) for t in 0:58])
# After extinction, the edges of species 49 should disappear.
topology(sol, 58).outgoing[49]
topology(sol, 59).outgoing[49] |
Hi @alaindanet @ismael-lajaaiti, and thank you for feedback :)
Agreed: that is a source of confusion. That method intersects two naming logics:
Maybe I should switch the whole solution methods suite back to
Yupe, why not, this is just a matter of making it a keyword parameter instead :)
Possible, although not exactly straightforward: the topology abstracts over all possible nodes/edges compartments, so you would have to specify which "adjacency" you actually need: That being said, I can definitely expose something like: get_adjacency(g::Topology, source_compartment, edge_compartment, target_compartment)::SparseMatrix{Bool} and then maybe wrap the few typical ones.. ? get_foodweb_adjacency(g::Topology) = get_adjacency(g, :species, :trophic, :species)
get_nutrients_adjacency(g::Topology) = get_adjacency(g, :species, :trophic, :nutrients) # (damn, this would include consumers)
get_facilitation_adjacency(g::Topology) = get_adjacency(g, :species, :facilitation, :species)
# ... .. unless maybe just leave Note that, if you know the extinct species, then removing the corresponding lines + columns from
Wops! The |
Sounds good to me.
Mmh.. I was suspecting that this was not as easy as I thought. IMO, it would be great to leave both options, that is, the 'raw' function and its wrappers.
OK... Then how can we manipulate a |
Trying to list the "wrappers": I can think of In the end, maybe this calls for too much API surface X) What about reducing all that with some simpler: # Lib.
get_adjacency(g, source, edge, target) = ...
get_species_adjacency(g, edge_type) = get_adjacency(g, :species, edge_type, :species)
get_foodweb_adjacency(g) = get_species_adjacency(g, :trophic)
# User.
A = get_foodweb_adjacency(g)
Af = get_species_adjacency(g, :facilitation)
Ar = get_species_adjacency(g, :refuge)
...
An = get_adjacency(g, :species, :trophic, :nutrients)[model.producers_mask, :] .. for now instead ?
Yay, well it depends on what you mean by "manipulate" ^ ^" Maybe you can get a sense of the inherent complexity of the
If you want to mean all of this implicitly, then maybe you need to work with As a reminder: the introduction of these
So, yeah, in the context of this PR (1), there is not much you can do with topologies yet except for detecting degenerated networks. In the future though, I would be glad to make them easy to query/split/export/whatever, but this is maybe out of scope for #152?.. .. unless you can write down the things you would like to do with them, and they would be easy to feature with small primitives like |
Yes, let's go for this for now.
OK, I forgot that you primarily designed the |
cfebf3d
to
f1b740d
Compare
Hi @ismael-lajaaiti @alaindanet. I think the last few commits address everything discussed so far here. Can you check whether it's okay? What do you think ? :) |
Everything seems to work good on my side. Thank you @iago-lito ! ERROR: ArgumentError: Invalid edge type label: :facilitation. Valid labels are :refuge and :trophic. I'm thinking of two things here:
For next, I also think we should discuss these points, let me know if they deserve their own issue:
|
Thank you for feedback @ismael-lajaaiti :)
I'm not sure we can do much better because the list of possible "interaction types" is open. Today, neither the topology or the model values know about all possible interaction types, and so there is no (simple) way they could make a difference between:
Thus the error message you are seing now. Maybe I can still clarify a little though: Invalid edge type label: :facilitation. Valid labels in this model are :refuge and :trophic. ..?
For the same reason: in principle, anyone could add the interaction type they want to the system by creating their own custom components (although it's (very) not ready yet). So given input like I'm curious though: what would you use this empty matrix for? How come your code needs to request facilitation links in a model without a
Oh, I thought these were guaranteed not to generate such degenerated networks. If they aren't, then of course we could feature that. The algorithm would just be bruteforce like the one to avoid cycles, right? Draw random networks until we find one without this kind of species.
Of course :) My first (modest) contribution was this comment. What I can do on my side is to start keeping track of API changes in a toplevel Unfortunately, I am leaving tonight for three weeks off. What I can do is merge this PR into See you about end of August <3 |
Yes, IMO this is already better!
OK, I see.
First, my reasoning was that a model without a
Yes, and yes. ;)
Enjoy your (well deserved) holidays! 🌴 ☀️ |
DifferentialEquations seems to have changed its default solver, which made extinction dates non-reproducible?
3cac9fd
to
eef9415
Compare
These values describe the model under a topological perspective: nodes and their neibouhring relations. Nodes and edges are typed into various 'compartments'. Nodes can be "removed" from topologies while leaving tombstones to maintain indices validity. This enables various topological analyses of the model network like `disconnected_components()`, `isolated_producers()` or `starving_consumers()`.
eef9415
to
ad4878d
Compare
That is.. very true. They are the same in principle, but they differ in the code because the former does not store extra facilitation data whereas the latter does. This is an unfortunate quirk.. :\ But I'm not sure what to do about it without also stating that "every model is a model with an empty
Okay, I see two approaches in this situations:
Would either be okay ? :) |
Start work on addressing #151.
[STATUS] The feature is complete and has landed on
dev
. Before merging intomain
, though:CHANGELOG.md
entry to land this intomain
as ourv0.2.1
:)