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

Adding priorities to UserDemand/Time causes initialization to fail #1250

Closed
Huite opened this issue Mar 13, 2024 · 1 comment · Fixed by #1268
Closed

Adding priorities to UserDemand/Time causes initialization to fail #1250

Huite opened this issue Mar 13, 2024 · 1 comment · Fixed by #1268
Assignees

Comments

@Huite
Copy link
Contributor

Huite commented Mar 13, 2024

Running a model with all UserDemand at 1 runs fine, but assigning (random) priorities 1 to 3 results in this error below.

I haven't changed anything else in the model, so allocation isn't used.
In that case, the whole grouping per priority should maybe not happen at all?

❯ c:\bin\ribasim\bin\ribasim.exe ribasim.toml
┌ Info: Starting a Ribasim simulation.
│   cli.ribasim_version = "2024.4.0"
│   starttime = 2017-01-01T00:00:00
└   endtime = 2019-01-01T00:00:00
ERROR: BoundsError: attempt to access 0-element Vector{Float64} at index [1]
Stacktrace:
  [1] getindex
    @ .\essentials.jl:13 [inlined]
  [2] get_scalar_interpolation(starttime::Dates.DateTime, t_end::Float64, time::StructArrays.StructVector{Ribasim.UserDemandTimeV1, @NamedTuple{node_id::Vector{Int64}, time::Vector{Dates.DateTime}, demand::Vector{Float64}, return_fa
ctor::Vector{Float64}, min_level::Vector{Float64}, priority::Vector{Int64}}, Int64}, node_id::Ribasim.NodeID, param::Symbol; default_value::Float64)
    @ Ribasim D:\buildAgent\work\ecd2b8f9b25b1609\ribasim\core\src\util.jl:215
  [3] Ribasim.UserDemand(db::SQLite.DB, config::Ribasim.config.Config)
    @ Ribasim D:\buildAgent\work\ecd2b8f9b25b1609\ribasim\core\src\read.jl:703
  [4] Ribasim.Parameters(db::SQLite.DB, config::Ribasim.config.Config)
    @ Ribasim D:\buildAgent\work\ecd2b8f9b25b1609\ribasim\core\src\read.jl:893
  [5] Ribasim.Model(config::Ribasim.config.Config)
    @ Ribasim D:\buildAgent\work\ecd2b8f9b25b1609\ribasim\core\src\model.jl:51
  [6] run(config::Ribasim.config.Config)
    @ Ribasim D:\buildAgent\work\ecd2b8f9b25b1609\ribasim\core\src\main.jl:11
  [7] (::Ribasim.var"#206#208"{Ribasim.config.Config})()
    @ Ribasim D:\buildAgent\work\ecd2b8f9b25b1609\ribasim\core\src\main.jl:66
  [8] with_logstate(f::Function, logstate::Any)
    @ Base.CoreLogging .\logging.jl:515
  [9] with_logger
    @ Base.CoreLogging .\logging.jl:627 [inlined]
 [10] #205
    @ Ribasim D:\buildAgent\work\ecd2b8f9b25b1609\ribasim\core\src\main.jl:59 [inlined]
 [11] open(::Ribasim.var"#205#207"{Ribasim.config.Config}, ::String, ::Vararg{String}; kwargs::@Kwargs{})
    @ Base .\io.jl:396
 [12] open
    @ Base .\io.jl:393 [inlined]
 [13] main(ARGS::Vector{String})
    @ Ribasim D:\buildAgent\work\ecd2b8f9b25b1609\ribasim\core\src\main.jl:56
 [14] main()
    @ Ribasim D:\buildAgent\work\ecd2b8f9b25b1609\ribasim\core\src\main.jl:24
 [15] top-level scope
    @ none:1
@Huite
Copy link
Contributor Author

Huite commented Mar 13, 2024

I reckon the problem is here:

        elseif node_id in time_node_ids
            push!(demand_from_timeseries, true)
            for p in priorities
                push!(demand, 0.0)
                if p in keys(time_priority_dict)
                    demand_p_itp, is_valid = get_scalar_interpolation(
                        config.starttime,
                        t_end,
                        time_priority_dict[p],
                        node_id,
                        :demand;
                        default_value = 0.0,
                    )
                    if is_valid
                        push!(demand_itp_node_id, demand_p_itp)
                    else
                        @error "The demand(t) relationship for UserDemand #$node_id of priority $p from the time table has repeated timestamps, this can not be interpolated."
                        errors = true
                    end
                else
                    demand_p_itp = LinearInterpolation([0.0, 0.0], trivial_timespan)
                    push!(demand_itp_node_id, demand_p_itp)
                end
            end
            push!(demand_itp, demand_itp_node_id)

In my case, I have a node 118, with priority 3.

We first make the time_priority_dict. In this case, priority 1 => (data for node 131, 132).
We iterate over the node_ids. 118 is first. It fetches the data for priority 1 first, with time_priority_dict[p]. This obviously doesn't contain the data for node 118. This then results in zero-sized vector inside of get_scalar_interpolation, which causes the index error.

I think line if p in keys(time_priority_dict) is a thinko? This should check for the presence of the node_id in the group instead.

In general, my feeling is the entire function UserDemand is in need of some restructuring. It's over 110 lines.
Static data should probably be dealt with in a separate function, and the transient data in another function.
Also not clear (haven't read super carefully to be fair): why are the data processed in order of priority?
If this really matters, it would be good to have some comments.

@elinenauta elinenauta moved this from To do to Sprint backlog in Ribasim Mar 14, 2024
Huite pushed a commit that referenced this issue Mar 18, 2024
@github-project-automation github-project-automation bot moved this from Sprint backlog to ✅ Done in Ribasim Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants