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

Jump to bottom

Allow multiple outputs at OffsetConverter #1053

Closed
lensum opened this issue Feb 16, 2024 · 4 comments · Fixed by #1054
Closed

Allow multiple outputs at OffsetConverter #1053

lensum opened this issue Feb 16, 2024 · 4 comments · Fixed by #1054

Comments

@lensum
Copy link
Contributor

lensum commented Feb 16, 2024

For a project, I need to model a Converter with part-load depending efficiencies that outputs two flows. This is currently not possible with the OffsetConverter. I propose the following changes:

  1. outputs should be allowed to contain multiple busses.
  2. coefficients should expect a dictionary, where the keys are the output busses and the values are the tuples containing the necessary parameters.
  3. The issues discussed in Misleading/missing documentation of OffsetConverter #1010 should be fixed (misleading docu, usage guide and erroneous example).
@lensum
Copy link
Contributor Author

lensum commented Feb 16, 2024

I think the PR or the existing issue are better places for discussion (if necessary), therefore I'm closing this again.

@lensum lensum closed this as completed Feb 16, 2024
@p-snft
Copy link
Member

p-snft commented Feb 20, 2024

Thanks for bringing this up. IMHO, you should have both, a PR discussing the implementation and an issue discussing the general idea.

About the general idea: There are two possible options to implement something like this. Multiple inputs or multiple outputs. You cannot have both. So, wouldn't it be simpler to just use two OffsetConverters and one Converter? This allows the flexibility to do both.

@p-snft p-snft reopened this Feb 20, 2024
@p-snft p-snft linked a pull request Feb 20, 2024 that will close this issue
Multiple Outputs for OffsetConverter #1054
Merged
@lensum
Copy link
Contributor Author

lensum commented Feb 20, 2024

If I understand you correctly, you mean that the Converter would effectively duplicate the incoming flow (by setting the conversion_factor to 1 for each output) and these are then fed into the OffsetConverter? That might work as well... It would also increase the number of variables, but maybe the pre-solve of efficient solvers would take care of that.

@p-snft
Copy link
Member

p-snft commented Feb 21, 2024

You would have additional variables but code that is a lot easier to maintain. Also, it is a lot more flexible, e.g. allows to combine OffsetConverter and PicewiseLinearConverter.

@p-snft p-snft mentioned this issue May 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Multiple Outputs for OffsetConverter lensum/oemof-solph
2 participants
@p-snft @lensum