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

Periodic value deserialization #124

Merged
merged 19 commits into from
Oct 16, 2023
Merged

Conversation

nailend
Copy link
Collaborator

@nailend nailend commented Aug 24, 2023

close #115

if multi-period approach is taken (periods resource exists):

  • facade parameters are checked if list and length equals number of periods
  • a full time series is generated with period specific values

This is only possible for certain parameters, otherwise building the energy system fails.

  • warning is raised every time periodic values are converted to a timeseries

Attention:

The investment parameters work differently. One period specific value is enough, and a whole timeseries is not necessary. Check here

e.g. "fixed_costs" "capacity_costs"

These period values are handled differently:

  • no timeseries is created
  • only list of values for every period are passed

The period information is needed to create the list of periodic
values. Therefore the reading has to take place beforehand.
If a list length equals the number of periods it is converted
to a timeseries of total timeindex length. The value will change
corresponding to the periods.
@nailend nailend self-assigned this Aug 24, 2023
@nailend nailend requested a review from henhuy August 24, 2023 18:46
@nailend
Copy link
Collaborator Author

nailend commented Oct 4, 2023

Change in oemof.solph issue 983 might have an effect on this

Copy link
Collaborator

@henhuy henhuy left a comment

Choose a reason for hiding this comment

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

Minor change requested. Otherwise this looks good to me.

src/oemof/tabular/datapackage/reading.py Outdated Show resolved Hide resolved
src/oemof/tabular/datapackage/reading.py Outdated Show resolved Hide resolved
@nailend
Copy link
Collaborator Author

nailend commented Oct 11, 2023

@henhuy not sure what's happening here and why the tests fail. Locally, there is no error ... Could you please check locally as well?

@henhuy
Copy link
Collaborator

henhuy commented Oct 12, 2023

I could reproduce error with latest python version 3.9.18 - in 3.9.16 error did not occur.
Could not figure out what has been changed in between those versions, though.
Error comes up, as Flow component "has" an attribute _in_edges, but for some reasons the attribute cannot be accessed and therefor throws an error:
https://github.com/oemof/oemof-solph/blob/f02297ea7db0134bfff12c3ff6c7c909ca4405ed/src/oemof/solph/processing.py#L515-L532
Fix could be to reverse checking:

attrs = [
    i
    for i in dir(com)
    if not (i.startswith(exclusions) or callable(getattr(com, i)))
]

We could implement this fix in oemof.solph - but first we would have to add a failing test to show and explain what's happening.

@nailend
Copy link
Collaborator Author

nailend commented Oct 12, 2023

Strange... python version 3.9.18 is from August'23 and was also already used in this CI run which wasn't failing.

@nailend
Copy link
Collaborator Author

nailend commented Oct 12, 2023

I found it. It's caused by changes in oemof.network apparently version 0.5.0a1 was still working.
This might be of interest to you, @p-snft ?

@p-snft
Copy link
Member

p-snft commented Oct 12, 2023

Unfortunately, network has some untested and undocumented parts. I did a release that did not really change anything to the "official" API.

I already reversed the checks that @henhuy mentioned in dev branch of solph: oemof/oemof-solph@559c681. Probably, a release makes sense as well.

@p-snft
Copy link
Member

p-snft commented Oct 12, 2023

There now is oemof.solph 0.5.2.dev0 that should fix the issue.

@nailend nailend merged commit dacedac into dev Oct 16, 2023
2 checks passed
@nailend nailend deleted the features/periodic-value-deserialization branch October 16, 2023 15:04
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.

Add multi-period parameters to deserialization
3 participants