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.

Sign up for GitHub

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

Jump to bottom

Fix/storage costs for initial level #1094

Merged
merged 3 commits into from
Aug 19, 2024
Merged

Fix/storage costs for initial level #1094

merged 3 commits into from
Aug 19, 2024

Conversation

p-snft
Copy link
Member

@p-snft p-snft commented Jul 31, 2024 •
edited
Loading

  • For optimisation using an assumed cyclic time, it does not make sense to consider both, the 0th and N+1st time point. This can be seen in particular as the storage is typically balanced, so it is really one value that gains twice the weight.
  • If time is assumed linear (no balanced storage), there are N+1 points in time for N time steps. Thus, the implementation makes sense to me. However, typically the initial content is predefined, so it just gives an offset to the objective value.

Thus, I suggest to drop the costs for the 0th time step.

Targeted for the next "major" release, because it can change the behaviour in some cases.

@p-snft
Simplify storage costs constraint
305cd2b 
The costs are defined on TIMEPOINTS, so these should be used.
@p-snft
Ignore storage_costs for 0th time point
ba99a2d 
In the constraint test, as the initial_storage_level was given, only a
constant was dropped. Thus, costs for the initial level never made it to
the lp file. I changed the costs to increase over time in the test to make
this more transpatrent.
@p-snft p-snft linked an issue Jul 31, 2024 that may be closed by this pull request
N+1 timesteps regarded in storage_costs for GenericStorage #1091
Closed
@p-snft
Copy link
Member Author

p-snft commented Aug 1, 2024

Coverage slightly decreases because I delete lines of tested code.

@p-snft p-snft marked this pull request as ready for review August 1, 2024 07:31
@p-snft p-snft added this to the v0.6.0 milestone Aug 1, 2024
@p-snft p-snft requested a review from a team August 2, 2024 06:05
@p-snft p-snft self-assigned this Aug 13, 2024
jokochems
jokochems approved these changes Aug 18, 2024
View reviewed changes
Copy link
Member

@jokochems jokochems left a comment

Choose a reason for hiding this comment

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

Thanks. Looks sound to me @p-snft.

@p-snft p-snft merged commit 2d02102 into dev Aug 19, 2024
13 of 14 checks passed
@p-snft p-snft deleted the fix/storage_costs-for-initial-level branch August 19, 2024 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jokochems jokochems jokochems approved these changes

Assignees

@p-snft p-snft

Labels
enhancement oemof-solph
Projects
None yet
Milestone
major release
Development

Successfully merging this pull request may close these issues.

N+1 timesteps regarded in storage_costs for GenericStorage
2 participants
@p-snft @jokochems