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

Add length check (setting it) for sequences #1117

Merged
merged 10 commits into from
Aug 29, 2024
Merged

Add length check (setting it) for sequences #1117

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c5cd8a4
Add length check (seting it) for sequences
p-snft Aug 26, 2024
d7887e0
Merge branch 'release/v0.5' into fix/length-of-fake-sequences-unset
p-snft Aug 26, 2024
3e66f74
Add test for valid_sequence(...)
p-snft Aug 26, 2024
03e0c9f
Add test for an invalid sequence
p-snft Aug 26, 2024
164d96a
Update docstrings of _plumbing
p-snft Aug 26, 2024
0c394ed
Add _FakeSequence fix to changelog
p-snft Aug 26, 2024
71af2ba
Delete ignored data from integration tests
p-snft Aug 26, 2024
32f9857
Warn about sequences that are too long
p-snft Aug 26, 2024
f7e72ae
Test for long sequence warning
p-snft Aug 26, 2024
f6f5339
Forbid valid_sequence to overwrite existing length
p-snft Aug 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/changelog.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ These are new features and improvements of note in each release
:backlinks: top


.. include:: whatsnew/v0-5-5.rst
.. include:: whatsnew/v0-5-4.rst
.. include:: whatsnew/v0-5-3.rst
.. include:: whatsnew/v0-5-2.rst
Expand Down
12 changes: 12 additions & 0 deletions docs/whatsnew/v0-5-5.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
v0.5.5 ()
--------------------------

Bug fixes
#########

* Fix iterating over _FakeSequence objects

Contributors
############

* Patrik Schönfeldt
25 changes: 23 additions & 2 deletions src/oemof/solph/_plumbing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@

def sequence(iterable_or_scalar):
"""Checks if an object is iterable (except string) or scalar and returns
the original sequence if object is an iterable and an 'emulated'
sequence object of class _Sequence if object is a scalar or string.
the an numpy array of the sequence if object is an iterable or an
'emulated' sequence object of class _FakeSequence if object is a scalar.

Parameters
----------
Expand Down Expand Up @@ -56,6 +56,27 @@ def sequence(iterable_or_scalar):
return _FakeSequence(value=iterable_or_scalar)


def valid_sequence(sequence, length: int) -> bool:
"""Checks if an object is a numpy array of at least the given length
or an 'emulated' sequence object of class _FakeSequence.
The latter is set to the required lenght.

"""
if sequence[0] is None:
return False

if isinstance(sequence, _FakeSequence):
sequence.size = length
return True
Copy link
Member

@uvchik uvchik Aug 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I would check it differently:

if isinstance(sequence, _FakeSequence):
    if sequence.size is None:
        return True
    else: 
        if sequence.size == length:
            return True
        else:
            return False

For me a sequence fits for any size, therefore, it is always TRUE, regardless of the checked length.

This behaviour will change if you set a length. From this moment on the size is set. If you now check the wrong length it is not valid any more.

I do not like the hidden setter. I would not expect the function to set the size of the sequence.
If you really want this hidden magic, I would prefer the following:

if isinstance(sequence, _FakeSequence):
    if sequence.size is None:
        sequence.size = length
        return True
    else: 
        if sequence.size == length:
            return True
        else:
            return False

This will fail if I ask for a different length later. In your function the size will be updated, even if the check ask for different length.

Copy link
Member Author

@p-snft p-snft Aug 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I understand what you are saying. Still, I tried to avoid this being a "hidden setter": According to the docstring, valid_sequence explicitly makes the argument a valid sequence of the given length. I did this to solve #1115: In your results, you might want to iterate over all values, real and fake sequences, in the same way.

I see three ways of doing this:

  1. Proceed as suggested in this PR.
  2. Rename the function valid_sequence to something like assure_valid_sequence to emphasize possible changes by the function.
  3. Make this even more explicit, e.g. by
    a. using the explicit setter (_Fakesequence.size = length) when needed, or by
    b. renaming valid_sequence to make_valid_sequence. This would then raise an Error instead of returning False.
  4. Completely remove the _FakeSequence and keep scalars. Then, cast on demand (to an np.array).

I'd opt to do something quick for the v0.5 release (so that the old behavior is maintained) and to remove the _FakeSequence in v0.6.

if isinstance(sequence, np.ndarray):
if sequence.size >= length:
return True
else:
p-snft marked this conversation as resolved.
Show resolved Hide resolved
raise ValueError(f"Lentgh of {sequence} should be {length}")

p-snft marked this conversation as resolved.
Show resolved Hide resolved
return False


class _FakeSequence:
"""Emulates a list whose length is not known in advance.

Expand Down
6 changes: 3 additions & 3 deletions src/oemof/solph/components/_extraction_turbine_chp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
from pyomo.environ import BuildAction
from pyomo.environ import Constraint

from oemof.solph._plumbing import sequence as solph_sequence
from oemof.solph.components._converter import Converter
from oemof.solph._plumbing import sequence
from oemof.solph.components import Converter


class ExtractionTurbineCHP(Converter):
Expand Down Expand Up @@ -87,7 +87,7 @@ def __init__(
custom_attributes=custom_attributes,
)
self.conversion_factor_full_condensation = {
k: solph_sequence(v)
k: sequence(v)
for k, v in conversion_factor_full_condensation.items()
}

Expand Down
33 changes: 15 additions & 18 deletions src/oemof/solph/components/_generic_storage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@

from oemof.solph._helpers import check_node_object_for_missing_attribute
from oemof.solph._options import Investment
from oemof.solph._plumbing import sequence as solph_sequence
from oemof.solph._plumbing import sequence
from oemof.solph._plumbing import valid_sequence


class GenericStorage(Node):
Expand Down Expand Up @@ -224,19 +225,15 @@ def __init__(

self.initial_storage_level = initial_storage_level
self.balanced = balanced
self.loss_rate = solph_sequence(loss_rate)
self.fixed_losses_relative = solph_sequence(fixed_losses_relative)
self.fixed_losses_absolute = solph_sequence(fixed_losses_absolute)
self.inflow_conversion_factor = solph_sequence(
inflow_conversion_factor
)
self.outflow_conversion_factor = solph_sequence(
outflow_conversion_factor
)
self.max_storage_level = solph_sequence(max_storage_level)
self.min_storage_level = solph_sequence(min_storage_level)
self.fixed_costs = solph_sequence(fixed_costs)
self.storage_costs = solph_sequence(storage_costs)
self.loss_rate = sequence(loss_rate)
self.fixed_losses_relative = sequence(fixed_losses_relative)
self.fixed_losses_absolute = sequence(fixed_losses_absolute)
self.inflow_conversion_factor = sequence(inflow_conversion_factor)
self.outflow_conversion_factor = sequence(outflow_conversion_factor)
self.max_storage_level = sequence(max_storage_level)
self.min_storage_level = sequence(min_storage_level)
self.fixed_costs = sequence(fixed_costs)
self.storage_costs = sequence(storage_costs)
self.invest_relation_input_output = invest_relation_input_output
self.invest_relation_input_capacity = invest_relation_input_capacity
self.invest_relation_output_capacity = invest_relation_output_capacity
Expand Down Expand Up @@ -603,7 +600,7 @@ def _objective_expression(self):

if m.es.periods is not None:
for n in self.STORAGES:
if n.fixed_costs[0] is not None:
if valid_sequence(n.fixed_costs, len(m.PERIODS)):
fixed_costs += sum(
n.nominal_storage_capacity
* n.fixed_costs[pp]
Expand All @@ -615,7 +612,7 @@ def _objective_expression(self):
storage_costs = 0

for n in self.STORAGES:
if n.storage_costs[0] is not None:
if valid_sequence(n.storage_costs, len(m.TIMESTEPS)):
storage_costs += (
self.storage_content[n, 0] * n.storage_costs[0]
)
Expand Down Expand Up @@ -1861,7 +1858,7 @@ def _objective_expression(self):
period_investment_costs[p] += investment_costs_increment

for n in self.INVESTSTORAGES:
if n.investment.fixed_costs[0] is not None:
if valid_sequence(n.investment.fixed_costs, len(m.PERIODS)):
lifetime = n.investment.lifetime
for p in m.PERIODS:
range_limit = min(
Expand All @@ -1879,7 +1876,7 @@ def _objective_expression(self):
)

for n in self.EXISTING_INVESTSTORAGES:
if n.investment.fixed_costs[0] is not None:
if valid_sequence(n.investment.fixed_costs, len(m.PERIODS)):
lifetime = n.investment.lifetime
age = n.investment.age
range_limit = min(
Expand Down
19 changes: 10 additions & 9 deletions src/oemof/solph/components/experimental/_sink_dsm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@

from oemof.solph._options import Investment
from oemof.solph._plumbing import sequence
from oemof.solph._plumbing import valid_sequence
from oemof.solph.components._sink import Sink


Expand Down Expand Up @@ -703,7 +704,7 @@ def _objective_expression(self):
* (1 + m.discount_rate) ** (-m.es.periods_years[p])
)

if g.fixed_costs[0] is not None:
if valid_sequence(g.fixed_costs, len(m.PERIODS)):
fixed_costs += sum(
max(g.max_capacity_up, g.max_capacity_down)
* g.fixed_costs[pp]
Expand Down Expand Up @@ -1434,7 +1435,7 @@ def _objective_expression(self):
* (1 + m.discount_rate) ** (-m.es.periods_years[p])
)

if g.investment.fixed_costs[0] is not None:
if valid_sequence(g.investment.fixed_costs, len(m.PERIODS)):
lifetime = g.investment.lifetime
for p in m.PERIODS:
range_limit = min(
Expand All @@ -1452,7 +1453,7 @@ def _objective_expression(self):
)

for g in self.EXISTING_INVESTDSM:
if g.investment.fixed_costs[0] is not None:
if valid_sequence(g.investment.fixed_costs, len(m.PERIODS)):
lifetime = g.investment.lifetime
age = g.investment.age
range_limit = min(
Expand Down Expand Up @@ -2198,7 +2199,7 @@ def _objective_expression(self):
* (1 + m.discount_rate) ** (-m.es.periods_years[p])
)

if g.fixed_costs[0] is not None:
if valid_sequence(g.fixed_costs, len(m.PERIODS)):
fixed_costs += sum(
max(g.max_capacity_up, g.max_capacity_down)
* g.fixed_costs[pp]
Expand Down Expand Up @@ -3290,7 +3291,7 @@ def _objective_expression(self):
* (1 + m.discount_rate) ** (-m.es.periods_years[p])
)

if g.investment.fixed_costs[0] is not None:
if valid_sequence(g.investment.fixed_costs, len(m.PERIODS)):
lifetime = g.investment.lifetime
for p in m.PERIODS:
range_limit = min(
Expand All @@ -3308,7 +3309,7 @@ def _objective_expression(self):
)

for g in self.EXISTING_INVESTDSM:
if g.investment.fixed_costs[0] is not None:
if valid_sequence(g.investment.fixed_costs, len(m.PERIODS)):
lifetime = g.investment.lifetime
age = g.investment.age
range_limit = min(
Expand Down Expand Up @@ -4391,7 +4392,7 @@ def _objective_expression(self):
* (1 + m.discount_rate) ** (-m.es.periods_years[p])
)

if g.fixed_costs[0] is not None:
if valid_sequence(g.fixed_costs, len(m.PERIODS)):
fixed_costs += sum(
max(g.max_capacity_up, g.max_capacity_down)
* g.fixed_costs[pp]
Expand Down Expand Up @@ -5791,7 +5792,7 @@ def _objective_expression(self):
* (1 + m.discount_rate) ** (-m.es.periods_years[p])
)

if g.investment.fixed_costs[0] is not None:
if valid_sequence(g.investment.fixed_costs, len(m.PERIODS)):
lifetime = g.investment.lifetime
for p in m.PERIODS:
range_limit = min(
Expand All @@ -5809,7 +5810,7 @@ def _objective_expression(self):
)

for g in self.EXISTING_INVESTDSM:
if g.investment.fixed_costs[0] is not None:
if valid_sequence(g.investment.fixed_costs, len(m.PERIODS)):
lifetime = g.investment.lifetime
age = g.investment.age
range_limit = min(
Expand Down
10 changes: 8 additions & 2 deletions src/oemof/solph/flows/_investment_flow_block.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
from pyomo.core import Var
from pyomo.core.base.block import ScalarBlock

from oemof.solph._plumbing import valid_sequence


class InvestmentFlowBlock(ScalarBlock):
r"""Block for all flows with :attr:`Investment` being not None.
Expand Down Expand Up @@ -1007,7 +1009,9 @@ def _objective_expression(self):
period_investment_costs[p] += investment_costs_increment

for i, o in self.INVESTFLOWS:
if m.flows[i, o].investment.fixed_costs[0] is not None:
if valid_sequence(
m.flows[i, o].investment.fixed_costs, len(m.PERIODS)
):
lifetime = m.flows[i, o].investment.lifetime
for p in m.PERIODS:
range_limit = min(
Expand All @@ -1022,7 +1026,9 @@ def _objective_expression(self):
)

for i, o in self.EXISTING_INVESTFLOWS:
if m.flows[i, o].investment.fixed_costs[0] is not None:
if valid_sequence(
m.flows[i, o].investment.fixed_costs, len(m.PERIODS)
):
lifetime = m.flows[i, o].investment.lifetime
age = m.flows[i, o].investment.age
range_limit = min(
Expand Down
40 changes: 32 additions & 8 deletions src/oemof/solph/flows/_non_convex_flow_block.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
from pyomo.core import Var
from pyomo.core.base.block import ScalarBlock

from oemof.solph._plumbing import valid_sequence


class NonConvexFlowBlock(ScalarBlock):
r"""
Expand Down Expand Up @@ -327,15 +329,19 @@ def _startup_costs(self):

if m.es.periods is None:
for i, o in self.STARTUPFLOWS:
if m.flows[i, o].nonconvex.startup_costs[0] is not None:
if valid_sequence(
m.flows[i, o].nonconvex.startup_costs, len(m.TIMESTEPS)
):
startup_costs += sum(
self.startup[i, o, t]
* m.flows[i, o].nonconvex.startup_costs[t]
for t in m.TIMESTEPS
)
else:
for i, o in self.STARTUPFLOWS:
if m.flows[i, o].nonconvex.startup_costs[0] is not None:
if valid_sequence(
m.flows[i, o].nonconvex.startup_costs, len(m.TIMESTEPS)
):
startup_costs += sum(
self.startup[i, o, t]
* m.flows[i, o].nonconvex.startup_costs[t]
Expand All @@ -361,15 +367,21 @@ def _shutdown_costs(self):

if m.es.periods is None:
for i, o in self.SHUTDOWNFLOWS:
if m.flows[i, o].nonconvex.shutdown_costs[0] is not None:
if valid_sequence(
m.flows[i, o].nonconvex.shutdown_costs,
len(m.TIMESTEPS),
):
shutdown_costs += sum(
self.shutdown[i, o, t]
* m.flows[i, o].nonconvex.shutdown_costs[t]
for t in m.TIMESTEPS
)
else:
for i, o in self.SHUTDOWNFLOWS:
if m.flows[i, o].nonconvex.shutdown_costs[0] is not None:
if valid_sequence(
m.flows[i, o].nonconvex.shutdown_costs,
len(m.TIMESTEPS),
):
shutdown_costs += sum(
self.shutdown[i, o, t]
* m.flows[i, o].nonconvex.shutdown_costs[t]
Expand All @@ -395,15 +407,21 @@ def _activity_costs(self):

if m.es.periods is None:
for i, o in self.ACTIVITYCOSTFLOWS:
if m.flows[i, o].nonconvex.activity_costs[0] is not None:
if valid_sequence(
m.flows[i, o].nonconvex.activity_costs,
len(m.TIMESTEPS),
):
activity_costs += sum(
self.status[i, o, t]
* m.flows[i, o].nonconvex.activity_costs[t]
for t in m.TIMESTEPS
)
else:
for i, o in self.ACTIVITYCOSTFLOWS:
if m.flows[i, o].nonconvex.activity_costs[0] is not None:
if valid_sequence(
m.flows[i, o].nonconvex.activity_costs,
len(m.TIMESTEPS),
):
activity_costs += sum(
self.status[i, o, t]
* m.flows[i, o].nonconvex.activity_costs[t]
Expand All @@ -429,15 +447,21 @@ def _inactivity_costs(self):

if m.es.periods is None:
for i, o in self.INACTIVITYCOSTFLOWS:
if m.flows[i, o].nonconvex.inactivity_costs[0] is not None:
if valid_sequence(
m.flows[i, o].nonconvex.inactivity_costs,
len(m.TIMESTEPS),
):
inactivity_costs += sum(
(1 - self.status[i, o, t])
* m.flows[i, o].nonconvex.inactivity_costs[t]
for t in m.TIMESTEPS
)
else:
for i, o in self.INACTIVITYCOSTFLOWS:
if m.flows[i, o].nonconvex.inactivity_costs[0] is not None:
if valid_sequence(
m.flows[i, o].nonconvex.inactivity_costs,
len(m.TIMESTEPS),
):
inactivity_costs += sum(
(1 - self.status[i, o, t])
* m.flows[i, o].nonconvex.inactivity_costs[t]
Expand Down
Loading
Loading