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

FakeSequences Not Iterating Correctly #1115

Closed
ahbills opened this issue Aug 23, 2024 · 4 comments · Fixed by #1117
Closed

FakeSequences Not Iterating Correctly #1115

ahbills opened this issue Aug 23, 2024 · 4 comments · Fixed by #1117
Assignees

Comments

@ahbills
Copy link

ahbills commented Aug 23, 2024

Description
With the changes in the new 0.5.4 update, I've run into a problem when trying to treat FakeSequences as an iterable. I have made the short code below to demonstrate:

from oemof.solph.components import Sink, Source
from oemof.solph.flows import Flow
import pandas as pd

es = EnergySystem(
    timeindex=create_time_index(2022, 1, 3),
    infer_last_interval=False
)

bus = Bus(label = 'Bus')
source = Source(
    label = 'Source',
    outputs={bus: Flow(variable_costs=[0.1, 0.3, 0.2])}
)
sink = Sink(
    label='Sink',
    inputs = {bus: Flow(fix = [25, 30, 50], nominal_value=100)}
)

es.add(bus, source, sink)

model = Model(es)

df= pd.DataFrame(
    {
        k:v.variable_costs
        for k,v in model.flows.items()
        if v.variable_costs is not None
    }
)

print(df)

Expected behavior
In the previous version, this prints the following:

   Bus Source
  Sink    Bus
0    0    0.1
1    0    0.3
2    0    0.2

but now the following error is raised

    fixes = pd.DataFrame(
            ^^^^^^^^^^^^^
  File "C:\venv\Lib\site-packages\pandas\core\frame.py", line 778, in __init__
    mgr = dict_to_mgr(data, index, columns, dtype=dtype, copy=copy, typ=manager)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\venv\Lib\site-packages\pandas\core\internals\construction.py", line 503, in dict_to_mgr
    return arrays_to_mgr(arrays, columns, index, dtype=dtype, typ=typ, consolidate=copy)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\venv\Lib\site-packages\pandas\core\internals\construction.py", line 114, in arrays_to_mgr
    index = _extract_index(arrays)
            ^^^^^^^^^^^^^^^^^^^^^^
  File "C:\venv\Lib\site-packages\pandas\core\internals\construction.py", line 662, in _extract_index
    raw_lengths.append(len(val))
                       ^^^^^^^^
TypeError: 'NoneType' object cannot be interpreted as an integer

and if you try to cast it to a list in the process: i.e. list(v.fix), the following error is raised

  File "c:\Examples\Bug_Ex.py", line 26, in <module>
    {
  File "c:\Examples\Bug_Ex.py", line 27, in <dictcomp>
    k:list(v.fix)
      ^^^^^^^^^^^
  File "C:\venv\Lib\site-packages\oemof\solph\_plumbing.py", line 104, in __iter__
    return repeat(self._value, self._length)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: 'NoneType' object cannot be interpreted as an integer

I believe that this is coming from a problem with the self._length value but I am having a hard time tracing it

Thank you in advance for any help

Note: I am working with:
oemof.solph 0.5.4
Python 3.11.9
pandas 2.2.2
and Windows 11

@p-snft p-snft added the bug label Aug 26, 2024
@p-snft p-snft self-assigned this Aug 26, 2024
@p-snft
Copy link
Member

p-snft commented Aug 26, 2024

Hi @ahbills,

thanks for reporting. Actually, I have to look a bit deeper before I understand why it worked before.

Why it does not work now, is rather clear: _FakeSequences are not iterable before a length is given explicitly. The old _Sequence class did some hidden magic to allow this. It memorized the last access using the [] operator and than iterated until that value. As a consequence, the behavior was depending on something that you would have expected to be read accesses. Currently, I do not see why there should be an access to flows[(Bus,Source)].variable_costs[2] in your code example.

I will report back once I checked,
Patrik

PS: The hidden magic making it hard to debug is the reason why we got rid of the old code.

@p-snft p-snft added the triaged label Aug 26, 2024
@p-snft
Copy link
Member

p-snft commented Aug 26, 2024

Found it. Flow.variable_costs defaults to 0. Thus,

if m.flows[i, o].variable_costs[0] is not None:
will be True also if no variable costs are set. With the old hidden magic _Sequence, the loop afterwards would access members Flow.variable_costs[0:3], and thus set the sequence length to 3.

Now, there are two thing we learn:

  1. The implementation of the FlowBlock (including the check) implies that a performance improvement was planned by not setting it. (At the moment, if you don't explicitly set variable_costs=None, the constraint will be build for every flow.) I think, mathematically setting zero by default makes sense. But this mean the check instead has to be if (m.flows[i, o].variable_costs[0] == 0).all(). Maybe, None was a better default.
  2. Even if None is the default, we need to set the length, as any scalar might be given (explicitly).

Let me put this as a question. Would the following be an acceptable (excepted without having the prior experience) result? (I think it would be consistent, as many other values default to None.)

    Source
    Bus
0   0.1
1   0.3
2   0.2

@ahbills
Copy link
Author

ahbills commented Aug 26, 2024

Wow thank you so much for all of your work! I think your suggested output is great.

Thank you,
Aidan

@p-snft
Copy link
Member

p-snft commented Aug 29, 2024

I just released solph v0.5.5 to fix that problem.

@p-snft p-snft closed this as completed Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants