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

Not all (pre-)calculations take DataFrames or Series as input #51

Open
jakob-wo opened this issue Jan 14, 2020 · 13 comments
Open

Not all (pre-)calculations take DataFrames or Series as input #51

jakob-wo opened this issue Jan 14, 2020 · 13 comments

Comments

@jakob-wo
Copy link
Contributor

jakob-wo commented Jan 14, 2020

I was ask if I could make the compression_heatpumps_and_chillers.py module take a Pandas.Series (time-indexed) as input and return the COPs as time-indexed Series.

Now I wonder, if that would be a good feature.
It seems to be very useful when using the module as stand-alone calculation but when using the calculation to generate input to a solph.component it has to be a List, as far as I know.

@jnnr
Copy link
Member

jnnr commented Jan 22, 2020

Which exact function do you mean? calc_cops() works fine with pd.Series, already:

import pandas as pd

from oemof.thermal.compression_heatpumps_and_chillers import calc_cops


periods = 24

datetimeindex = pd.date_range('2020-01-01', periods=periods, freq='H')

temp_h = pd.Series([60] * periods, index=datetimeindex)
temp_l = pd.Series([30] * periods, index=datetimeindex)
print(temp_h)

cops = calc_cops(temp_h, temp_l, 0.9, mode='heat_pump')

print(cops)

@jnnr
Copy link
Member

jnnr commented Jan 22, 2020

But it returns a list, not a Series. Is that what you meant?

@jakob-wo
Copy link
Contributor Author

yes, that's the point! You will get a list (without time-index) back from the function.

And I did not think about Series when I wrote the function just afterwards I found that it works as well. So when I implemented an input argument check (PR #57) I realized that I need to be aware of other types, as well, apart from list.

That is why I like to have a feedback if we want to make oemof.thermal (pre-)calculations capable to handle Series.

@jnnr
Copy link
Member

jnnr commented Jan 22, 2020

If we find an easy way to allow for Series and lists, that would be a nice feature.

@nesnoj
Copy link
Member

nesnoj commented Jan 22, 2020

... when using the calculation to generate input to a solph.component it has to be a List, as far as I know

Let me pick this up even though it's a little off-topic. I discussed this recently with @gplssm and @nailend. oemof's Flow() says numeric (e.g. in nominal_value) which I assume to be a builtin numeric type. This does not include a pandas Series.
However, it actually seems to work.. Should the docs be adjusted?

@jnnr
Copy link
Member

jnnr commented Jan 22, 2020

Are you sure that works with nominal value? I just tested it, and it fails with nominal_value as list or Series.

import pandas as pd

from oemof.solph import EnergySystem, Model, Sink, Transformer, Bus, Flow

datetimeindex = pd.date_range(start='2020-01-01', periods=3, freq='H')

es = EnergySystem(timeindex=datetimeindex)

bus_0 = Bus('bus_0', balanced=False)

bus_1 = Bus('bus_1')

nominal_value_trafo = 40
# nominal_value_trafo = [40, 40, 40]
# nominal_value_trafo = pd.Series([40, 40, 40])

transformer = Transformer(
    'transformer',
    inputs={bus_0: Flow()},
    outputs={bus_1: Flow(
        nominal_value=nominal_value_trafo,
        variable_costs=10)},
    conversion_factors={bus_0: 1}
)

sink = Sink(
    'sink',
    inputs={
        bus_1: Flow(
            nominal_value=1,
            actual_value=[1,2,3],
            fixed=True)
    }
)

es.add(bus_0, bus_1, transformer, sink)

model = Model(es)
model.solve()
print(model.results())

@jnnr
Copy link
Member

jnnr commented Jan 22, 2020

Other parameters like max accept both scalars and lists or series (numeric (sequence or scalar)) . Scalars are converted to sequences in the background (plumbing.py module).

@nesnoj
Copy link
Member

nesnoj commented Jan 23, 2020

Are you sure that works with nominal value?

Sorry, my bad. I just picked out one of the numeric inputs and this was the very one where it's not allowed. As you say, I mean the sequence type params such as actual_value and max - which include builtin type list, tuple and range (docs).

Apparently, all iterable objects are allowed here according to this line in sequence() (except for string).

You may check with

>>> from collections.abc import Iterable
>>> from pandas import Series
>>> isinstance(Series([1, 2, 3]), Iterable)
True

The docstring should be something like

...
max : numeric (sequence or scalar) - sequence must be iterable (instance of abc.Iterable) except str
...

@jnnr
Copy link
Member

jnnr commented Jan 23, 2020

The docstring should be something like

...
max : numeric (sequence or scalar) - sequence must be iterable (instance of abc.Iterable) except str
...

Do you think it would help to add sequence must be iterable (instance of abc.Iterable) except str to oemof's docstrings? In that case you should open an issue in oemof/oemof.

@nesnoj
Copy link
Member

nesnoj commented Jan 24, 2020

Done in https://github.com/oemof/oemof/issues/673
Close this one?

@jnnr
Copy link
Member

jnnr commented Jan 24, 2020

Thanks!
This issue should remain open, as the inital problem of pandas.Series not getting properly processed by the functions in oemof-thermal is not solved yet. I played around with some code that I can post next week.

@jnnr jnnr added this to the v0.0.2 milestone Mar 9, 2020
@FranziPl
Copy link
Member

Just for information, how it is handed in other oemof-thermal components:
The csp needs time indexed series, because the pv-lib, which is used, demands the time index. So in the end input and output has the same time index.

@c-moeller c-moeller removed this from the v0.0.2 milestone Mar 26, 2020
@jnnr jnnr added this to the v0.0.3 milestone Mar 26, 2020
@c-moeller c-moeller removed this from the v0.0.3 milestone Jun 17, 2020
@jakob-wo
Copy link
Contributor Author

jakob-wo commented Sep 8, 2020

This Issue seems not to be too relevant any more and I thnink we won't find time to work on this any more.
I propose to close this Issue. Any dissent?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants