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

Enhanc/Potential in links #47

Merged
merged 10 commits into from
Nov 6, 2024
Merged

Enhanc/Potential in links #47

merged 10 commits into from
Nov 6, 2024

Conversation

JulStraus
Copy link
Member

This PR focuses on increasing the flexibility of Links. It incorporates several different concepts for links to allow for

  1. differing input and output resources of links as well as specifying these directly (ae10474),
  2. bidirectional links and nodes (113b8f3),
  3. emissions of links (8110d8d),
  4. OPEX of links (with both fixed and variable OPEX created at the same time) (166fe57),
  5. capacity of links (9027a4d),
  6. inclusion of new variables (f8d959a), and
  7. investments in links (7c18c7e).

All introduced changes are only valid if the user specifies explicitly for their abstract or composite type that they should include them.

I did not update the documentation with additional information regarding the extension of Links yet. In practice, it only includes the new variables and the docstrings, but not how one can create new links.

- Relaxed lower bounds for flows
- Manual fix of the lower bounds of variables
- It always creates both fixed and variable
- Tested for both EMI and EMB, but to be extended in EMI tests later
- `constratins_capacity_installed` included for links
- Function called from `create_link`, but not used
- Requires the inclusion of a function `EMB.capacity(l, t)` for links
- Tested for OPEX with InvestmentModel
- Not directly incorporated into the model
@JulStraus JulStraus added the enhancement New feature or request label Oct 28, 2024
Copy link
Member

@hellemo hellemo left a comment

Choose a reason for hiding this comment

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

Looks good, this is an important step towards more unified building of models from both nodes and links.

I have thought a bit about the naming, and I currently lean towards using the same function for creating variables and constraints for nodes and links, and dispatch to different methods. Then we could get a nice top level loop that clearly communicats that we do the same for nodes and links (or where they differ), something like:

for elements  (𝒩, ℒ)
     variables_opex(m, elements, 𝒯, 𝒫, modeltype)
     variables_capex(m, elements, 𝒯, modeltype)
     variables_capacity(m, elements, 𝒯, modeltype)
     variables_nodes(m, elements, 𝒯, modeltype)
end

I suggest we follow that up in a separate PR.

ext/EMIExt/EMIExt.jl Show resolved Hide resolved
ext/EMIExt/objective.jl Outdated Show resolved Hide resolved
src/model.jl Outdated Show resolved Hide resolved
src/model.jl Outdated Show resolved Hide resolved
src/model.jl Outdated Show resolved Hide resolved
src/model.jl Outdated Show resolved Hide resolved
test/test_links.jl Outdated Show resolved Hide resolved
test/test_links.jl Outdated Show resolved Hide resolved
]

# Connect all nodes with the availability node for the overall energy/mass balance
links = [
Copy link
Member

Choose a reason for hiding this comment

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

I guess this is ok, but it's hard to read/make sense of and a pattern some users seen to copy from tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. In that respect, I think the people should focus instead on the examples. Note, that the changes in test_nodes.jl are mostly related to providing tests to existing functions (and the newly introduced unidirectional and bidirectional properties).

@JulStraus
Copy link
Member Author

Looks good, this is an important step towards more unified building of models from both nodes and links.

I have thought a bit about the naming, and I currently lean towards using the same function for creating variables and constraints for nodes and links, and dispatch to different methods. Then we could get a nice top level loop that clearly communicats that we do the same for nodes and links (or where they differ), something like:

for elements  (𝒩, ℒ)
     variables_opex(m, elements, 𝒯, 𝒫, modeltype)
     variables_capex(m, elements, 𝒯, modeltype)
     variables_capacity(m, elements, 𝒯, modeltype)
     variables_nodes(m, elements, 𝒯, modeltype)
end

I suggest we follow that up in a separate PR.

Generally speaking, I like the design of looping through everything. We could also split flow variables into two methods to be certain regarding the application. In theory, that should not be breaking as we do not export the functions.

There are however two things I want to stress:

  • variables_nodes would be slightly confusing if we iterate through with links. We should rename it. Potentially variables_elements to be consistent?
  • we should simultaneously consider which arguments we want to pass to the functions to be consistent. The only function we are currently exporting is variables_node which would be unaffected by the design. Hence, we can clean up the functions.

All of the changes should in my opinion be a separate PR, but with the same versions increase. That way, we do not introduce a version which we directly change again.

@JulStraus JulStraus merged commit adfb7a4 into main Nov 6, 2024
5 checks passed
@JulStraus JulStraus deleted the enhanc/link_description branch November 6, 2024 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants