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

Support "non-integer" delay for Connections #56

Merged
merged 3 commits into from
Feb 3, 2025
Merged

Conversation

daschw
Copy link
Collaborator

@daschw daschw commented Jan 30, 2025

This adds support for Connection delays that are not a multiple of snapshot weights.

Initially, I wanted to also support arbitrary, i.e. not necessarily equidistant snapshots. However, it turned out that this very easily results in infeasible models:
Just consider, for example, three snapshots with weights [2, 1, 1], a connection from node A to node B with a delay of two hours, and a demand at node B of [1, 2, 1]. There is no feasible solution of a connection injection at node A in the first snapshots that satisfies the demand at node B in the last two snapshots.

So, decided to not (yet or at all?) support different snapshot weights in the presence of connection delays.

cc: @GerhardTotschnig

@daschw daschw requested a review from sstroemer January 30, 2025 15:12
Copy link

codecov bot commented Jan 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 50.33%. Comparing base (57b3d60) to head (c637dc2).

@@            Coverage Diff             @@
##             main      #56      +/-   ##
==========================================
+ Coverage   50.25%   50.33%   +0.08%     
==========================================
  Files         109      109              
  Lines        4318     4319       +1     
==========================================
+ Hits         2170     2174       +4     
+ Misses       2148     2145       -3     
Files with missing lines Coverage Δ
src/core/connection.jl 47.52% <ø> (-1.54%) ⬇️
src/core/connection/var_flow.jl 75.47% <100.00%> (+3.13%) ⬆️

... and 1 file with indirect coverage changes

Copy link
Member

@sstroemer sstroemer 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 to me, feel free to merge afterwards.

So, decided to not (yet or at all?) support different snapshot weights in the presence of connection delays.

I'd very much argue for NOT supporting that at all. As hinted in #23 we want to finally support the Kotzur formulation (e.g., the MGA project of Perine & Max really needs that) which means we should get rid of different snapshot weights (they potentially complicate a lot of stuff and may result in unexpected shadow prices, so ... not a lot of reasonable applications anyways).

assets/examples/50_delayed_connections.iesopt.yaml Outdated Show resolved Hide resolved
test/src/examples.jl Outdated Show resolved Hide resolved
@daschw
Copy link
Collaborator Author

daschw commented Jan 31, 2025

Regarding ait-energy/iesopt#71: Do you think we should make the choice of the loss snapshot depend on connection.loss_mode, i.e. should we choose the delayed snapshot for loss_mode === :to?

@sstroemer
Copy link
Member

Regarding ait-energy/iesopt#71: Do you think we should make the choice of the loss snapshot depend on connection.loss_mode, i.e. should we choose the delayed snapshot for loss_mode === :to?

I'm not sure, but I think it's fine as is because:

  1. That only matters for a case of non-constant loss, which may happen for dynamic line ratings based on temperature (but those would have no delay) - but I don't really see any other immediate case.
  2. That would probably lead to more confusion in understanding it than just saying "it's always based on the side it's coming from".

delay is probably going to stay a parameter that is used rarely, so ...

@daschw daschw merged commit f356329 into ait-energy:main Feb 3, 2025
2 checks passed
@daschw daschw deleted the delay branch February 3, 2025 07:07
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.

2 participants