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

Add an option to overwrite cost attributes from the configuration file. #1532

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

tgilon
Copy link
Contributor

@tgilon tgilon commented Feb 6, 2025

Changes proposed in this Pull Request

This PR suggests to add an options to overwrite investment, lifetime, FOM, VOM, efficiency and fuel attributes from the configuration file. This mimics the existing capital and marginal cost behaviour.

Checklist

  • I tested my contribution locally and it works as intended.
  • Code and workflow changes are sufficiently documented.
  • Changed dependencies are added to envs/environment.yaml.
  • Changes in configuration options are added in config/config.default.yaml.
  • Changes in configuration options are documented in doc/configtables/*.csv.
  • Sources of newly added data are documented in doc/data_sources.rst.
  • A release note doc/release_notes.rst is added.

@tgilon tgilon marked this pull request as ready for review February 6, 2025 16:08
Copy link
Member

@fneum fneum left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -851,7 +851,7 @@ costs:
lifetime: 25
"CO2 intensity": 0
"discount rate": 0.07
# Marginal and capital costs can be overwritten
# Marginal and capital costs, investment, lifetime, FOM, VOM, efficiency and fuel can be overwritten
Copy link
Member

Choose a reason for hiding this comment

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

wondering if those overwrites could be packaged into

costs:
  overwrites:
    marginal_cost: ...

even though this would be a breaking change for marginal_cost and capital_cost.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is why I avoided it. But I had the same idea, to be honest. As you prefer

Copy link
Member

Choose a reason for hiding this comment

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

Let's hear another opinion @FabianHofmann @lkstrp

Copy link
Contributor

@FabianHofmann FabianHofmann Feb 18, 2025

Choose a reason for hiding this comment

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

I would prefer a dedicated section, too. and this breaking change would be "softened" by the json schema since the user will get a clear message that the old place is not supported anymore

Copy link
Member

Choose a reason for hiding this comment

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

Agree, that should go in overwrites. But I would leave it non breaking for now (just move new attributes to overwrites and break once we have the deprecation messages.

Also, shouldn't fill_values (defaults is a better name here) and overwrites then allow for the exact same keys?

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.

5 participants