Conversation
- CompoundResource is a super type for ResourcePotential - Any CompoundResource creates new varaibles potential_in and potential_out on nodes and links. - In junctions the pontentials are transferred by equality rather than summation (as for flow_in, link_in etc.) - This allows for transfers of voltages and pressures, along with a quantity (e.g. energy or material flows)
- added function `variables_flow_resource` to create new variables by dispatching on resource types, it is called inside `variables_flow` - added function `constraints_link_resource` to create new constraints by dispatching on resource types, it is called inside `create_link` - added function `res_types` to extract an array of unique resource types from an array of resources
- Added function for resouce type constratints on node-link coupling
that dispatch on resouce type
- Removed old functions on potential variables and constratints
- Resource vector is segmented into sub-vectors based on resource type - Constraint functions for flows can be dispatched on these sub-vectors - New function are created to say if EMB flow-variables should be added, which can be dispatched on resource type, default is true - Function is available for adding new variables that are specific for a resource type
- Avioids repeating constraints that are not dependent on resources (doesnt have resouce in the constraint index)
- Moved the constraint-function for special
resource constraint to create_element function
…ase.jl into enhance/transfer_energy_potentials
* Updated `res_types` and `res_types_seg` to take `Vector{<:Resource}` instead of `Array{<:Resource}` as input.
* Added missing tests for new resource functions
…ase.jl into enhance/transfer_energy_potentials
JulStraus
left a comment
JulStraus
left a comment
There was a problem hiding this comment.
These two comments are related to potential breaking changes which should be revised.
* Revert changes for create_link * `total_duration` is added to SimpleTimes, and the docstring is updated to reflect this.
|
Resolves #61 |
JulStraus
left a comment
JulStraus
left a comment
There was a problem hiding this comment.
It looks in general quite well, but there must be the following changes:
- a description regarding how one can incorporate the new feature. This should be included within the how-to section and
- a proper test set in which we check that the specific functions are called and can be worked on.
We also have to discuss whether we want to have it breaking or not. There is a potential to avoid breaking changes, as outlined by me.
| res_types_seg(𝒫::Vector{<:Resource}) | ||
|
|
||
| Return a Vector-of-Vectors of resources segmented by the sub-types. | ||
| """ | ||
| res_types_seg(𝒫::Vector{<:Resource}) = [Vector{rt}(filter(x -> isa(x, rt), 𝒫)) for rt in res_types(𝒫)] No newline at end of file |
There was a problem hiding this comment.
Is that always guaranteed, that you receive a Vector{Vector{}}. I experienced some issues when we changed to the new Case structure.
I would suggest renaming the function to res_types_vec.
There was a problem hiding this comment.
No, it does not if the input is empty. Then it returns an empty Vector{Any}, but that is ok. Changed to check that it returns a empty vector for a empty input vector instead
| for 𝒳 ∈ 𝒳ᵛᵉᶜ | ||
| variables_capacity(m, 𝒳, 𝒳ᵛᵉᶜ, 𝒯, modeltype) | ||
| variables_flow(m, 𝒳, 𝒳ᵛᵉᶜ, 𝒯, modeltype) | ||
| variables_flow(m, 𝒳, 𝒳ᵛᵉᶜ, 𝒫, 𝒯, modeltype) |
There was a problem hiding this comment.
This change is a breaking change.
I am not opposed to it as I do not really see another approach, but I think it is beneficial to think whether we want to include additional breaking changes.
The breaking change could be circumvented by adding the following default method:
variables_flow(m, 𝒳::Vector{<:AbstractElement}, 𝒳ᵛᵉᶜ, 𝒫, 𝒯, modeltype::EnergyModel) =
variables_flow(m, 𝒳, 𝒳ᵛᵉᶜ, 𝒯, modeltype)I tested it with EnergyModelsGeography and it worked.
| - `Node` - the subfunction is [`create_node`](@ref). | ||
| - `Link` - the subfunction is [`create_link`](@ref). | ||
| """ | ||
| create_element(m, n::Node, 𝒯, 𝒫, modeltype::EnergyModel) = |
There was a problem hiding this comment.
The docstring should be updated for this function as we now actually have specific approaches. This also implies that we want to avoid people to create new methods for create_element as the function constraints_resource is now called within create_element.
This would also be stressed in the documentation.
There was a problem hiding this comment.
You do not test that a new Resource is actually implemented properly with all the different functions called. You can take a look how it implemented it for links. The current test set does not really test that the new functionality is working, except for the utility functions. I would however think, it would be beneficial to test properly that the system is working the way it should.
2 participants
Create new functions that enable dispatch on resource types in extension packages. This allows for new variables and constraints on existing elements when they are combined with specific resource types. New variables must have a unique name for each combination of element and resource. Couple functions must also be defined for the coupling between two elements for each resource.