Skip to content
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

Improve how computations are made over MTGs #51

Merged
merged 97 commits into from
Nov 23, 2023
Merged

Improve how computations are made over MTGs #51

merged 97 commits into from
Nov 23, 2023

Conversation

VEZY
Copy link
Member

@VEZY VEZY commented Sep 8, 2023

See #38.

The features that we need to have:

  1. (already there) associate a set of models to a symbol using a ModelList
  2. get access to variables from other scales if we need to
  3. make this access fully transparent without the need to understand how MultiScaleTreeGraph works or where the values of a variable are. I.e., access the variable using status.A, not e.g. node[:models].status[timestep][:carbon_demand].

To do:

  • 1. We can now provide a mapping between organs and models. This mapping is then used to compute a multiscale dependency graph.
  • For 2. and 3., we need to cache the tree traversal into an object, but not the nodes, directly the ModelLists, or even better, the Status. Problems:
    • These statuses are dynamic, as new organs can appear on the plant and older organs can die off. So each time the MTG is updated (adding or removing an organ), we must update this structure accordingly (see fa528e1).
    • We usually expect a singleton when doing status.var, but if we ask variables from several organs at a time (e.g. assimilation from all leaves in the plant), we will get a vector. As stated before, the number of organs can change over time, so we should always provide a vector of values because a singleton can become a vector dynamically, which is a nightmare to handle. Another way to manage all this is to pass the values via the extra field, but in this case, we would lose the ability to traverse the mtg from inside a model. This is not acceptable as we use it for adding/removing organs. OK so the solution is to use vectors inside the Status. Those vectors should be typed (we know the types, so it's OK, and ideally, they shouldn't copy the value but be a Vector of Ref instead. => This is done, we now pass the variables as RefVectors to other scales if the user map the variables as e.g. :var => ["Leaf"], and as a singleton if the mapping is e.g. :var => "Leaf". So in the end, it is the user that tells us if only one organ is expected during the simulation.
  • Include a test on models when the model has theIsObjectIndependent()trait. In this case, it should check that variables from other scales are given as input to the model.

update doc as it now uses a cache variable
@codecov
Copy link

codecov bot commented Sep 8, 2023

Welcome to Codecov 🎉

Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment.

Thanks for integrating Codecov - We've got you covered ☂️

@VEZY
Copy link
Member Author

VEZY commented Sep 8, 2023

OK so what I did is to implement RefVector, which is a structure that holds a vector of references to values in a vector of Status. This way we don't make copies, and if we update the value from this vector, it will also update the value in the status.

Fix error in eltype(::Status)
Remove unnecessary argument typing in parse_models
Add mutating functions
[First draft]
Still working on the initialisation
Make a status for each node based on the template dict + references between variables
Make the status_template in one pass (with  link between scales)
Add first step of `init_simulation`: just the statuses, not the dependency graph yet
Add doc for init_simulation
Remove schema (waiting for better way from PlantMeteo)
Add example mtg
Fix issues when a variable has different default values between scales (uses the upper-stream model now) and fix an issue for mapping to variables with one node only
Now uses a sub-module for the examples
Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead). See: JuliaLang/julia#49553
Now that we changed how we make simulations over MTGs, we remove the old tests
export processes
Reduce the size of this help file, by removing some outputs
increase size_threshold to 250kB.
This is done so we can dynamically add organs
@VEZY VEZY merged commit 48dd4f1 into main Nov 23, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant