Feature/custom attributes for investments · Pull Request #1174 · oemof/oemof-solph

ghost · 2025-03-12T18:10:27Z

Constraints Investments for costume attributes missed the possibility of constraining not only the investment flow, but the sum of investment and operation. Furthermore no OffSet for costume Attributes in investments were considers.

This PR adds both and explains it in an example.

It is handy to have these to run a restricted optimization, as explained in Issue 1171

examples/generic_invest_limit/example_generic_invest_offset.py

…ribute add tsam logic and weighting to additional total limit of costume attribute

p-snft

At the moment, I would see this as a Showcase entry. Sure, the constraint is written in a generic way, but an example works without tests and discussions about the API.

(@gnn and I were discussing how to harmonise different types of variables, so that a "flow of money" would be constrained using the same code as energy flows. But maybe, this would rather be something for solph 2.)

p-snft · 2025-08-26T10:58:51Z

examples/generic_invest_limit/example_generic_invest_offset.py

+
+Installation requirements
+-------------------------
+This example requires oemof.solph (v0.5.x), install by:


Really solph 0.5?

examples/generic_invest_limit/example_generic_invest_offset.py

model.GenericInvestmentStorageBlock.invest[st, p] * getattr(storages[st][0].investment, keyword).get("cost", 0) gets always calculated 2 times in the constraint. Idk where bug comes from.

…e offset was always set

p-snft

Thanks for your efforts. I see the following issues that should be addressed before a merge:

The approach seems to duplicate a lot of additional_investment_flow_limit - which it extends. From my point of view, we do not really need both. So, you might just extend the existing constraint, rename it and provide a transitional wrapper. (You can have keyword arguments to still only consider flows and no offsets.)
The constraint you introduce is completely untested.

Maxhi77 · 2025-09-18T13:54:30Z

Thank you for the feedback.

To specify the offset or cost per unit of a custom attribute, I used the following:

custom_attributes={“space”: {“cost”: 1, “offset”: 5}},

The old version used instead:

custom_attributes={"space": 1}

This is relevant at the moment the constraint is set. If I put a wrapper around the old constraint, it only works if

I check the structure of the custom attributes and look for cost or not. I don't like this idea.
It would be easier to simply change the structure of the custom_attributes to the new one.
Keep the old restriction and write a note that the new structure must be used in the future.

p-snft · 2025-09-18T14:33:04Z

This is relevant at the moment the constraint is set. If I put a wrapper around the old constraint, it only works if ...

I see the point. As this is not about about arguments to the function only, a classical wrapper will not do. Maybe, just write a deprecation warning for the old constraint, telling that the new one should be used in the future. In that case, I'd suggest a test that proves that both constraints provide the same result if custom_attributes={"space": {"cost": 1}} (and no offset is given.)

PS: The name "cost" might be misleading. Maybe, just be mathematical here and call them "linear" and "offset".

…ional flows

…mprove desc.

…vestments' into feature/custom_attributes_for_investments # Conflicts: # examples/emission_constraint/emission_constraint_invest.py # examples/emission_constraint/emission_constraint_invest_and_flow.py

p-snft · 2025-09-22T13:57:15Z

examples/emission_constraint/emission_constraint_invest_and_flow.py

+import logging
+import os
+
+from PIL.ImageChops import offset


I also do not get why you import this.

examples/emission_constraint/emission_constraint_invest_and_flow.py

+from oemof import solph
+
+
+def main(optimize=True):


p-snft · 2025-09-22T13:57:01Z

tests/test_constraints/test_additional_total_limit.py

+"""
+
+import pandas as pd
+import pytest


How is this fixed?

tests/test_constraints/test_additional_total_limit.py

Maxhi77 · 2025-09-19T09:52:48Z

Thank you very much for the feedback.

I have implemented all the requested changes. I've also moved all examples that restrict a costume attribute to an example folder and renamed it Emission. Why this? I think this will be much easier for people to find. Also, there is now a better and more consistent structure in the examples with limits of: flow, invest, invest and flow.

p-snft

I did not repeat the CodeQL comments, but two of those were (and still are) valuable.

p-snft · 2025-09-22T13:57:01Z

tests/test_constraints/test_additional_total_limit.py

+"""
+
+import pandas as pd
+import pytest


How is this fixed?

p-snft · 2025-09-22T13:57:15Z

examples/emission_constraint/emission_constraint_invest_and_flow.py

+import logging
+import os
+
+from PIL.ImageChops import offset


I also do not get why you import this.

Maxhi77 added 5 commits March 12, 2025 00:52

add additional_total_limit(inv+oper) for costume attributes

d1c9dc1

modify example to offset of costume attribute

3f4c460

add init of additional_total_limit method

654ec81

add GenericStorage to total_costume_attribute constraint

a65c747

change texting/doc

c407a66

ghost requested a review from p-snft March 12, 2025 18:10

github-advanced-security bot found potential problems Mar 12, 2025

View reviewed changes

examples/generic_invest_limit/example_generic_invest_offset.py Fixed Show fixed Hide fixed

examples/generic_invest_limit/example_generic_invest_offset.py Fixed Show fixed Hide fixed

examples/generic_invest_limit/example_generic_invest_offset.py Fixed Show fixed Hide fixed

add tsam logic and weighting to additional total limit of costume att…

262c0fb

…ribute add tsam logic and weighting to additional total limit of costume attribute

p-snft reviewed Aug 26, 2025

View reviewed changes

IMPORTANT: Weird bug quick and dirty solution in invest status

553ade8

model.GenericInvestmentStorageBlock.invest[st, p] * getattr(storages[st][0].investment, keyword).get("cost", 0) gets always calculated 2 times in the constraint. Idk where bug comes from.

p-snft marked this pull request as draft September 3, 2025 09:00

Maxhi77 and others added 7 commits September 17, 2025 13:34

Merge branch 'dev' into feature/custom_attributes_for_investments

f73bd30

reset quick and dirty solution, which was needed since generic storag…

3f3e8dd

…e offset was always set

example for offset customattributes

d0c6ac0

rename because only flow gets limited and not inv.

8c0fd9b

correct black error

070c9c4

correct black error

adeb188

Fix wrong docstring include in example_generic_invest

4c498ee

Maxhi77 requested review from gnn and p-snft September 17, 2025 14:57

Maxhi77 self-assigned this Sep 17, 2025

Fix docstring of example_generic_invest_offset

7ed222c

p-snft requested changes Sep 17, 2025

View reviewed changes

Maxhi77 added 5 commits September 19, 2025 10:59

implement deprication warning into "old" flow limit constraint

6790895

rename emission constraint to flow, because it only limits the operat…

95671ad

…ional flows

rename and move invest constraint to emission

3bb7e6d

improve post-processing of results to give better understanding and i…

fce2d87

…mprove desc.

run package black for format

8b5bef6

Maxhi77 added 3 commits September 19, 2025 11:34

add testing for new constraint

1ba7ae3

Merge remote-tracking branch 'origin/feature/custom_attributes_for_in…

4279c81

…vestments' into feature/custom_attributes_for_investments # Conflicts: # examples/emission_constraint/emission_constraint_invest.py # examples/emission_constraint/emission_constraint_invest_and_flow.py

change solver to cbc

7bdab63

github-advanced-security bot found potential problems Sep 19, 2025

View reviewed changes

imrpove bec of flake errors

09b0d52

Maxhi77 requested a review from p-snft September 19, 2025 12:48

p-snft reviewed Sep 22, 2025

View reviewed changes

Maxhi77 and others added 3 commits January 14, 2026 16:59

Update investment flow so it works with 1 period TSAM

8bbb075

correct generic stprage

c779be2

delete one syntax error, one , too much

1a33411

Conversation

ghost commented Mar 12, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

p-snft left a comment

Choose a reason for hiding this comment

Uh oh!

p-snft Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

p-snft left a comment

Choose a reason for hiding this comment

Uh oh!

Maxhi77 commented Sep 18, 2025

Uh oh!

p-snft commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Check notice

p-snft Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Check notice

Check notice

p-snft Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Maxhi77 commented Sep 19, 2025

Uh oh!

p-snft left a comment

Choose a reason for hiding this comment

Uh oh!

p-snft Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

p-snft Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

p-snft commented Sep 18, 2025 •

edited

Loading