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

Refactoring of postprocessing #73

Open
henhuy opened this issue Oct 7, 2022 · 2 comments
Open

Refactoring of postprocessing #73

henhuy opened this issue Oct 7, 2022 · 2 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@henhuy
Copy link
Collaborator

henhuy commented Oct 7, 2022

Hi @jnnr,

I revisited the postprocessing code, here my suggestions for refactoring the module:

  • helper functions look very good, only some minor refactoring possible but not prio 1 (i.e. get_losses and subtract_output_from_input could be improved)
  • Actual calculations in run_postprocessing could be refactored using my suggestion from https://gist.github.com/henhuy/2f94df3f80dd5c8b01c739218df9f82e
  • I would refactor code to only make use of str-based keys (instead of oemof.solph components) - this is needed i.e. in digiplan as data is sored/restored from DB in str-based format and postprocessing is done afterwards. Also we get rid of those nasty pickles!
  • change end of script in order to support multiple output formats (calculations are fine until every thing is gathered - then some oemof-B3 specific output formatting is done). Here I could think of a adapter-like interface for individual output formats

If we agree on refactoring, I would start by adding tests for postprocessing, so that we can guarantee results to be the same!

What do you think?

@henhuy henhuy added the enhancement New feature or request label Oct 7, 2022
@henhuy henhuy self-assigned this Oct 7, 2022
@jnnr
Copy link
Collaborator

jnnr commented Oct 10, 2022

Hi @henhuy, thanks for the initiative!

* helper functions look very good, only some minor refactoring possible but not prio 1 (i.e. `get_losses` and `subtract_output_from_input` could be improved)

Good to hear we can reuse them.

* Actual calculations in `run_postprocessing` could be refactored using my suggestion from https://gist.github.com/henhuy/2f94df3f80dd5c8b01c739218df9f82e

First impression: Looks great. I like the dependency handling.

* I would refactor code to only make use of str-based keys (instead of oemof.solph components) - this is needed i.e. in digiplan as data is sored/restored from DB in str-based format and postprocessing is done afterwards. Also we get rid of those nasty pickles!

Makes a lot of sense. Good to avoid pickling. Must ensure though that no additional information is lost that is hidden in the components.

* change end of script in order to support multiple output formats (calculations are fine until every thing is gathered - then some oemof-B3 specific output formatting is done). Here I could think of a adapter-like interface for individual output formats

Sounds good.

If we agree on refactoring, I would start by adding tests for postprocessing, so that we can guarantee results to be the same!

Great.

Some postprocessing steps are specific to a certain facade class (e.g. some result about electric vehicles). Do you have an idea how to handle those? Postprocessing methods could be incorporated in the facade. Postprocessing could e.g. check the "type" attribute which is related to the facade classes via facades.TYPEMAP and call these methods. Have not thought it through, thoug.

@jnnr
Copy link
Collaborator

jnnr commented Oct 10, 2022

Here are my notes on oemoflex' results from earller this month:

  • results should provide complete information, such that postprocessing can happen directly after optimization
  • results should be providable according to facade type or all together stacked. Probably already implemented (_get_scalars(by_element=True))
  • Both scalars and sequences results should be available either labeled by oemof-tuples and single strings. Currently, oemoflex maps scalar results: oemof tuples to flow_out_bus. But it does not map sequences.

TODOs

@jnnr jnnr added this to the v0.0.2 milestone Oct 12, 2022
@MaGering MaGering modified the milestones: v0.0.2, v0.0.3 Aug 31, 2023
@MaGering MaGering modified the milestones: v0.0.3, v0.0.4 Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants