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

New input files format - split of Generators_data.csv #612

Merged
merged 45 commits into from
Feb 15, 2024

Conversation

lbonaldo
Copy link
Collaborator

@lbonaldo lbonaldo commented Jan 13, 2024

Description

This PR splits Generators_data.csv into a few different files, one for each type of generator. This allows each file to contain only the attributes related to the specific type of generator for which it is meant.
A large number of files had to be changed, because all the examples and tests needed to be modified (and so did the associated csv files).

The implementation follows an initial restructuring proposed by @cfe316. The main formulation is defined in the resources.jl file and can be summarized as follows:

  • A new Resources folder located inside the case folder contains the new input files, one for each type of resource (e.g., vre.csv, storage.csv, hydro.csv, etc.), along with the files that contain the columns related to policies (e.g., Resource_capacity_reserve_margin.csv, Resource_energy_share_requirement.csv etc.)
  • load_resources_data.jl substitutes load_generators_data.jl and contains the functions that read each input file and create the new data structures
  • each resource is an instance of a Julia type (e.g., GenX.Vre, GenX.Hydro, GenX.Storage, etc). The types are defined in the resources.jl file along with a set of function interfaces to interact with the resources. In this way, attributes of each resource can be looked up indirectly through the interface or directly with the usual dot notation.
  • The interface also contains a set of utility functions that can be used to wrap operations that are used several times in the code. For example, the function resources_in_zone_by_rid(gen, zone) proposed by @cfe316 can be called instead of the way it has been done till now, i.e. using dfGen[dfGen.Zone .== zone, :R_ID] to get the ids of the resources in a certain zone. (Julia macros can be used to facilitate the creation of these function interfaces).

Since this PR is a significant change to the code, I'd appreciate any comments, thoughts, or suggestions on how to modify or improve it.

Todo:

  • Test RealSystemExample
  • Final cleanup
  • Update docs

What type of PR is this? (check all applicable)

  • Code Refactor

Checklist

  • Code changes are sufficiently documented; i.e. new functions contain docstrings and .md files under /docs/src have been updated if necessary.
  • The latest changes on the target branch have been incorporated, so that any conflicts are taken care of before merging. This can be accomplished either by merging in the target branch (e.g. 'git merge develop') or by rebasing on top of the target branch (e.g. 'git rebase develop'). Please do not hesitate to reach out to the GenX development team if you need help with this.
  • Code has been tested to ensure all functionality works as intended.
  • CHANGELOG.md has been updated (if this is a 'notable' change).
  • I consent to the release of this PR's code under the GNU General Public license.

How this can be tested

  • With the tests and the examples provided in the GenX repository.

Post-approval checklist for GenX core developers

After the PR is approved

  • Check that the latest changes on the target branch are incorporated, either via merge or rebase
  • Remember to squash and merge if incorporating into develop

@lbonaldo lbonaldo added this to the v0.4 milestone Jan 13, 2024
@lbonaldo lbonaldo force-pushed the feature-inputfiles branch 3 times, most recently from 0784cd4 to 586f270 Compare January 24, 2024 00:31
@lbonaldo
Copy link
Collaborator Author

This PR is now ready for review.

@lbonaldo lbonaldo marked this pull request as ready for review January 24, 2024 00:59
Copy link
Collaborator

@sambuddhac sambuddhac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are a few of my clarifying questions, @lbonaldo

  1. The exporting of AbstractResource in line 27 in GenX.jl has been added to facilitate the use of the newly defined abstract type at splitting the Generators_data.csv according to resources?
  2. I need some help understanding the modifications to piecewise fuel file, MGA, Method of Morris, and Multistage loading of inputs. I checked the other files following the guidelines from you and rest everything seems okay by me.
  3. I also tested this branch of the fork manually with a few example cases (real system three zone, real system one zone, simple system one zone, simple system three zone, and multistage real three zone and it works good for all of these except for the multistage model (Error message attached at the end of this message; I couldn't dig for debugging yet though).
  4. Can you please clarify once again the advantage of using the function calls instead of the dataframe referencing?

Error message from multistage real ISONE trizone:


***********
Iteration Number: 1
Upper Bound = Inf
Lower Bound = 64958.37577086839
***********
***********
Forward Pass t = 2
***********
Running HiGHS 1.6.0: Copyright (c) 2023 HiGHS under MIT licence terms
Presolving model
Problem status detected on presolve: Infeasible
Model   status      : Infeasible
Objective value     :  0.0000000000e+00
HiGHS run time      :          0.53
ERROR:   No LP invertible representation for getDualRay
MILP solved for primal
ERROR: LoadError: Result index of attribute MathOptInterface.VariablePrimal(1) out of bounds. There are currently 0 solution(s) in the model.
Stacktrace:
  [1] check_result_index_bounds
    @ ~/.julia/packages/MathOptInterface/DDWnF/src/attributes.jl:207 [inlined]
  [2] get
    @ ~/.julia/packages/HiGHS/k1fEq/src/MOI_wrapper.jl:2131 [inlined]
  [3] get(b::MathOptInterface.Bridges.LazyBridgeOptimizer{HiGHS.Optimizer}, attr::MathOptInterface.VariablePrimal, index::MathOptInterface.VariableIndex)
    @ MathOptInterface.Bridges ~/.julia/packages/MathOptInterface/DDWnF/src/Bridges/bridge_optimizer.jl:1265
  [4] get(model::MathOptInterface.Utilities.CachingOptimizer{MathOptInterface.Bridges.LazyBridgeOptimizer{HiGHS.Optimizer}, MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}, attr::MathOptInterface.VariablePrimal, index::MathOptInterface.VariableIndex)
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/DDWnF/src/Utilities/cachingoptimizer.jl:932
  [5] _moi_get_result(::MathOptInterface.Utilities.CachingOptimizer{MathOptInterface.Bridges.LazyBridgeOptimizer{HiGHS.Optimizer}, MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}, ::MathOptInterface.VariablePrimal, ::Vararg{Any})
    @ JuMP ~/.julia/packages/JuMP/HjlGr/src/optimizer_interface.jl:663
  [6] get(model::JuMP.Model, attr::MathOptInterface.VariablePrimal, v::JuMP.VariableRef)
    @ JuMP ~/.julia/packages/JuMP/HjlGr/src/optimizer_interface.jl:703
  [7] value(v::JuMP.VariableRef; result::Int64)
    @ JuMP ~/.julia/packages/JuMP/HjlGr/src/variables.jl:1703
  [8] value
    @ ~/.julia/packages/JuMP/HjlGr/src/variables.jl:1702 [inlined]
  [9] #55
    @ ./none:0 [inlined]
 [10] iterate
    @ ./generator.jl:47 [inlined]
 [11] _all(f::Base.var"#343#345", itr::Base.Generator{Vector{JuMP.VariableRef}, GenX.var"#55#56"}, #unused#::Colon)
    @ Base ./reduce.jl:1242
 [12] all
    @ ./reduce.jl:1238 [inlined]
 [13] Dict(kv::Base.Generator{Vector{JuMP.VariableRef}, GenX.var"#55#56"})
    @ Base ./dict.jl:131
 [14] fix_integers(jump_model::JuMP.Model)
    @ GenX ~/code/GenXPRTest/GenX/src/model/solve_model.jl:24
 [15] solve_model(EP::JuMP.Model, setup::Dict{Any, Any})
    @ GenX ~/code/GenXPRTest/GenX/src/model/solve_model.jl:73
 [16] run_ddp(models_d::Dict{Any, Any}, setup::Dict{Any, Any}, inputs_d::Dict{Any, Any})
    @ GenX ~/code/GenXPRTest/GenX/src/multi_stage/dual_dynamic_programming.jl:222
 [17] run_genx_case_multistage!(case::String, mysetup::Dict{Any, Any}, optimizer::Type)
    @ GenX ~/code/GenXPRTest/GenX/src/case_runners/case_runner.jl:145
 [18] run_genx_case!(case::String, optimizer::Type)
    @ GenX ~/code/GenXPRTest/GenX/src/case_runners/case_runner.jl:23
 [19] run_genx_case!(case::String)
    @ GenX ~/code/GenXPRTest/GenX/src/case_runners/case_runner.jl:17
 [20] top-level scope
    @ ~/code/GenXPRTest/GenX/Example_Systems/RealSystemExample/ISONE_Trizone_MultiStage/Run.jl:3
 [21] include(fname::String)
    @ Base.MainInclude ./client.jl:476
 [22] top-level scope
    @ REPL[4]:1
in expression starting at /Users/sc87/code/GenXPRTest/GenX/Example_Systems/RealSystemExample/ISONE_Trizone_MultiStage/Run.jl:3

caused by: Result index of attribute MathOptInterface.VariablePrimal(1) out of bounds. There are currently 0 solution(s) in the model.
Stacktrace:
  [1] check_result_index_bounds
    @ ~/.julia/packages/MathOptInterface/DDWnF/src/attributes.jl:207 [inlined]
  [2] get
    @ ~/.julia/packages/HiGHS/k1fEq/src/MOI_wrapper.jl:2131 [inlined]
  [3] get(b::MathOptInterface.Bridges.LazyBridgeOptimizer{HiGHS.Optimizer}, attr::MathOptInterface.VariablePrimal, index::MathOptInterface.VariableIndex)
    @ MathOptInterface.Bridges ~/.julia/packages/MathOptInterface/DDWnF/src/Bridges/bridge_optimizer.jl:1265
  [4] get(model::MathOptInterface.Utilities.CachingOptimizer{MathOptInterface.Bridges.LazyBridgeOptimizer{HiGHS.Optimizer}, MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}, attr::MathOptInterface.VariablePrimal, index::MathOptInterface.VariableIndex)
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/DDWnF/src/Utilities/cachingoptimizer.jl:932
  [5] _moi_get_result(::MathOptInterface.Utilities.CachingOptimizer{MathOptInterface.Bridges.LazyBridgeOptimizer{HiGHS.Optimizer}, MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}, ::MathOptInterface.VariablePrimal, ::Vararg{Any})
    @ JuMP ~/.julia/packages/JuMP/HjlGr/src/optimizer_interface.jl:663
  [6] get(model::JuMP.Model, attr::MathOptInterface.VariablePrimal, v::JuMP.VariableRef)
    @ JuMP ~/.julia/packages/JuMP/HjlGr/src/optimizer_interface.jl:703
  [7] value(v::JuMP.VariableRef; result::Int64)
    @ JuMP ~/.julia/packages/JuMP/HjlGr/src/variables.jl:1703
  [8] value
    @ ~/.julia/packages/JuMP/HjlGr/src/variables.jl:1702 [inlined]
  [9] #55
    @ ./none:0 [inlined]
 [10] iterate
    @ ./generator.jl:47 [inlined]
 [11] Dict{JuMP.VariableRef, Float64}(kv::Base.Generator{Vector{JuMP.VariableRef}, GenX.var"#55#56"})
    @ Base ./dict.jl:103
 [12] dict_with_eltype
    @ ./abstractdict.jl:575 [inlined]
 [13] dict_with_eltype
    @ ./abstractdict.jl:582 [inlined]
 [14] Dict(kv::Base.Generator{Vector{JuMP.VariableRef}, GenX.var"#55#56"})
    @ Base ./dict.jl:129
 [15] fix_integers(jump_model::JuMP.Model)
    @ GenX ~/code/GenXPRTest/GenX/src/model/solve_model.jl:24
 [16] solve_model(EP::JuMP.Model, setup::Dict{Any, Any})
    @ GenX ~/code/GenXPRTest/GenX/src/model/solve_model.jl:73
 [17] run_ddp(models_d::Dict{Any, Any}, setup::Dict{Any, Any}, inputs_d::Dict{Any, Any})
    @ GenX ~/code/GenXPRTest/GenX/src/multi_stage/dual_dynamic_programming.jl:222
 [18] run_genx_case_multistage!(case::String, mysetup::Dict{Any, Any}, optimizer::Type)
    @ GenX ~/code/GenXPRTest/GenX/src/case_runners/case_runner.jl:145
 [19] run_genx_case!(case::String, optimizer::Type)
    @ GenX ~/code/GenXPRTest/GenX/src/case_runners/case_runner.jl:23
 [20] run_genx_case!(case::String)
    @ GenX ~/code/GenXPRTest/GenX/src/case_runners/case_runner.jl:17
 [21] top-level scope
    @ ~/code/GenXPRTest/GenX/Example_Systems/RealSystemExample/ISONE_Trizone_MultiStage/Run.jl:3
 [22] include(fname::String)
    @ Base.MainInclude ./client.jl:476
 [23] top-level scope
    @ REPL[4]:1

Copy link
Collaborator

@sambuddhac sambuddhac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code reviewed, doubts clarified, successfully tested against both multistage and single stage one zone and three zone cases, and approved !!!

Copy link
Collaborator

@cfe316 cfe316 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Luca, I've made some minor comments - mostly style.

Comment on lines 490 to 510


resource_attribute_not_set() = 0

"""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be removed. It's also in load_resources_data and you can use default_zero there too.

Comment on lines 557 to 566
# Reservoir hydro and storage
const default_percent = 1.0
efficiency_up(r::T) where T <: Union{Hydro,Storage} = get(r, :eff_up, default_percent)
efficiency_down(r::T) where T <: Union{Hydro,Storage} = get(r, :eff_down, default_percent)

# Ramp up and down
const VarPower = Union{Electrolyzer, Hydro, Thermal}
min_power(r::VarPower) = get(r, :min_power, default_zero)
ramp_up_percentage(r::VarPower) = get(r, :ramp_up_percentage, default_percent)
ramp_down_percentage(r::VarPower) = get(r, :ramp_dn_percentage, default_percent)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know these are all called "percent" but they should all really be called "fraction" or "factor". If it's a percent it should go from 0 to 100. These all go from 0 to 1.

Generally I like to use "factor" for things which can be greater than 1. Fraction generally implies that it's less than a whole.

cluster(r::AbstractResource) = r.cluster

# UTILITY FUNCTIONS for working with resources
is_LDS(rs::Vector{T}) where T <: AbstractResource = findall(r -> get(r, :lds, default_zero) > 0, rs)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to make is_LDS defined for == 1 only.

is_LDS(rs::Vector{T}) where T <: AbstractResource = findall(r -> get(r, :lds, default_zero) > 0, rs)
is_SDS(rs::Vector{T}) where T <: AbstractResource = findall(r -> get(r, :lds, default_zero) == 0, rs)

ids_with_mga(rs::Vector{T}) where T <: AbstractResource = findall(r -> mga(r) > 0, rs)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to make ids_with_mga defined for == 1 only.

ids_with_spinning_reserve_requirements(rs::Vector{T}) where T <: AbstractResource = findall(r -> rsv_max(r) > 0, rs)

# Maintenance
ids_with_maintenance(rs::Vector{T}) where T <: AbstractResource = findall(r -> get(r, :maint, default_zero) > 0, rs)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can make ids_with_maintenance for == 1 rather than > 0.

Comment on lines +710 to +737
Returns the indices of all co-located solar resources in the vector `rs`.
"""
solar(rs::Vector{T}) where T <: AbstractResource = findall(r -> isa(r,VreStorage) && r.solar != 0, rs)

"""
wind(rs::Vector{T}) where T <: AbstractResource

Returns the indices of all co-located wind resources in the vector `rs`.
"""
wind(rs::Vector{T}) where T <: AbstractResource = findall(r -> isa(r,VreStorage) && r.wind != 0, rs)

"""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure the best way to fix this yet, but it might be confusing to readers if solar and wind only pick up VreStorage resources and not Vres.

Comment on lines 891 to 894
function resources_by_names(rs::Vector{AbstractResource}, names::Vector{String})
return rs[findall(r -> resource_name(r) ∈ names, rs)]
end

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this function is ever used ... delete it.

@@ -13,7 +13,7 @@ function thermal!(EP::Model, inputs::Dict, setup::Dict)
THERM_NO_COMMIT = inputs["THERM_NO_COMMIT"]
THERM_ALL = inputs["THERM_ALL"]

dfGen = inputs["dfGen"]
gen = inputs["RESOURCES"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line 16 is here twice (also 7)!

!isnothing(findfirst(r -> get(r, col, 0) ≠ 0, rs))
end


@doc raw"""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utilities/by_rid_df is now unused; it can be deleted.

@cfe316
Copy link
Collaborator

cfe316 commented Feb 12, 2024

I've now tested out the maintenance formulation. Getting it to work required these two changes in thermal_commit.jl.

Line 320: by_rid(y, :Cap_Size) should be by_rid(y, :cap_size) or cap_size(gen[y])
Line 376/377: cap_size = cap_size(gen[y]) creates a problem because the name is doubled... just rename to cap = cap_size(gen[y])

@lbonaldo
Copy link
Collaborator Author

lbonaldo commented Feb 14, 2024

Thanks everyone for the comments and suggestions! I have addressed them in the last version.

@lbonaldo lbonaldo merged commit 448d724 into GenXProject:develop Feb 15, 2024
5 checks passed
@lbonaldo lbonaldo deleted the feature-inputfiles branch June 25, 2024 16:39
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.

3 participants