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

Check on model dependency not working #77

Closed
VEZY opened this issue Jul 16, 2024 · 1 comment · Fixed by #83
Closed

Check on model dependency not working #77

VEZY opened this issue Jul 16, 2024 · 1 comment · Fixed by #83

Comments

@VEZY
Copy link
Member

VEZY commented Jul 16, 2024

There is no error when computing the initialisations when a model has a hard-dependency on another model declared with the dep function, e.g.:

PlantSimEngine.dep(::LeafStateModel) = (leaf_rank=AbstractLeaf_RankModel,)

If we don't give an AbstractLeaf_RankModel to the model list, there is no error raised, but we expect one!

Concrete example using XPalm:

using XPalm, PlantSimEngine, DataFrames, CSV

mapping = Dict(
    "Phytomer" => (
        XPalm.SexDetermination(),
        Status(TT_since_init = 10000.0, carbon_demand_plant=10.0, carbon_offer_plant=8.0)
    )
)

to_initialize(mapping)

outs = Dict{String,Any}(
    "Phytomer" => (:sex,),
)

meteo = CSV.read(joinpath(dirname(dirname(pathof(XPalm))), "0-data/meteo.csv"), DataFrame)
p = Palm(nsteps=nrow(meteo))

sim = run!(p.mtg, mapping, meteo, outputs=outs, executor=SequentialEx());

The model list would need the XPalm.ReproductiveOrganEmission model, but it is not given. So it should return some information about that when calling to_initialize, at least a warning. But here it works and we catch the error in the simulation when calling run! and get the following error:

`ERROR: type NamedTuple has no field reproductive_organ_emission``

I understand the error, but most users won't.

Note that it works if we call dep(mappin) instead:

julia> dep(mapping)
[ Info: Model SexDetermination from process sex_determination at scale Phytomer needs a model that is a subtype of XPalm.AbstractReproductive_Organ_EmissionModel in process reproductive_organ_emission, but the process is not parameterized in the ModelList.
╭──── Dependency graph ───────────────────────╮
│  ╭──── "Phytomer" => :sex_dete... ───────╮  │
│  │  ╭──── Main model ─────────────────╮  │  │
│  │  │  Process: sex_determination     │  │  │
│  │  │  Model: XPalm.SexDetermination  │  │  │
│  │  │  Dep: nothing                   │  │  │
│  │  ╰─────────────────────────────────╯  │  │
│  ╰───────────────────────────────────────╯  │
╰─────────────────────────────────────────────╯
The dependency graph is acyclic.
@VEZY
Copy link
Member Author

VEZY commented Jul 18, 2024

Also one more comment, we don't take into account the multi-scale aspect of model dependencies. Some model dependencies are defined at another scale. This is the case for example for the ReproductiveOrganEmission model in XPalm. The model needs an AbstractFinal_Potential_BiomassModel model as a dependency, but it is in fact one that is computed at the organ scale, so we should check if that model is present at the given scale. The call to the model in the function is here:

https://github.com/PalmStudio/XPalm.jl/blob/4a4dafb5a4ff012ff8ac4e85f553d0dfee06f0bf/src/plant/phytomer/phytomer/add_reproductive_organ.jl#L46

The model is searched in the sim_object object here: sim_object.models[status.sex].final_potential_biomass.

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 a pull request may close this issue.

1 participant