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

Feature/max cap ii #243

Merged
merged 23 commits into from
Apr 16, 2020
Merged

Feature/max cap ii #243

merged 23 commits into from
Apr 16, 2020

Conversation

Piranias
Copy link
Collaborator

@Piranias Piranias commented Apr 6, 2020

Continue on #237

Changes proposed in this pull request:
Add parameter "maximumCap" for

  • transformers (energyConversion.csv)
  • storages (e.g. storage_01.csv)

The following steps were realized, as well (if applies):

  • Use in-line comments to explain your code
  • Write docstrings to your code
  • For new functionalities: Explain in readthedocs
  • Write test(s) for your new patch of code
  • Update the CHANGELOG.md
  • Apply black (black . --exclude docs/)
  • Check if tests pass

Please mark above checkboxes as following:

  • Open
  • Done

❌ Check not applicable to this PR

For more information on how to contribute check the CONTRIBUTING.md.

@Piranias Piranias mentioned this pull request Apr 6, 2020
9 tasks
@Piranias
Copy link
Collaborator Author

Piranias commented Apr 6, 2020

Also this way D1 would be simplified again because all sources would have the parameter.

I am not sure what you refer to as source, if you need feedback, can you quote the line here?

If all assets that call the functions: source_dispatchable_optimize() and source_non_dispatchable_optimize() have the parameter "maximumCap" then the distinction "if "maximumCap" in dict_asset" in D1 would be redundant. This way we could simplify D1 again.

For transformers and storages I added a line in C1 that creates the parameter with value "None" if it doesn't exist yet to avoid the distinction in D1.

So my question is: are there other assets (e.g. the DSO) that call the functions source_dispatchable_optimize() and source_non_dispatchable_optimize() that do not have the parameter "maxiumCap"?

@smartie2076
Copy link
Collaborator

Also this way D1 would be simplified again because all sources would have the parameter.

I am not sure what you refer to as source, if you need feedback, can you quote the line here?

If all assets that call the functions: source_dispatchable_optimize() and source_non_dispatchable_optimize() have the parameter "maximumCap" then the distinction "if "maximumCap" in dict_asset" in D1 would be redundant. This way we could simplify D1 again.

Ah yeah, we should do that when creating the next MVS release that solves the compability issue. If you do not integrate below solution, can you create an issue that reminds us of that task?

For transformers and storages I added a line in C1 that creates the parameter with value "None" if it doesn't exist yet to avoid the distinction in D1.

Beautiful way to go - if that is easily done for the sources as well (pr #237), you could do the same here. In that case you do not have to create above issue, as there is nothing to be reminded of! :)

So my question is: are there other assets (e.g. the DSO) that call the functions source_dispatchable_optimize() and source_non_dispatchable_optimize() that do not have the parameter "maxiumCap"?

Yes, actually, you need to add the default value for "maximumCap" in this function, and probably here.
That function creates imaginary sources for peak demand pricing of the DSOs. I would not put it into the DSOs csvs, as that info might be confused with the transformer stations that have to be defined in energyConversion.csv.

Please run following test data to check whether or not the peak demand pricing function still works after you change the code (this one uses monthly peak pricing):
inputs.zip

@Piranias
Copy link
Collaborator Author

Piranias commented Apr 8, 2020

Yes, actually, you need to add the default value for "maximumCap" in this function, and probably here.
That function creates imaginary sources for peak demand pricing of the DSOs. I would not put it into the DSOs csvs, as that info might be confused with the transformer stations that have to be defined in energyConversion.csv.

I added a line to "define_source()" that creates the maxiumumCap parameter with "None" value and adapted D1 accordingly. I found that the function"define_source()" is only used to create DSO sources. Therefore I would suggest to change its name to "define_dso_source()" instead. It's kind of confusing the way it is, because powerplants are sources too. Or is there other cases where this function would be used for other sources than the dso?

Please run following test data to check whether or not the peak demand pricing function still works after you change the code (this one uses monthly peak pricing):
inputs.zip

actually these inputs did not work for me. But the error has nothing to do with my changes as far as I understand. I get an error in this line.

I could look where it comes from if no one else is working on that (apparently the dictionary "dict_values["energyProviders"][dso]" is not created correctly. But that would be another issue.

@smartie2076
Copy link
Collaborator

I added a line to "define_source()" that creates the maxiumumCap parameter with "None" value and adapted D1 accordingly.

Good.

I found that the function"define_source()" is only used to create DSO sources. Therefore I would suggest to change its name to "define_dso_source()" instead. It's kind of confusing the way it is, because powerplants are sources too. Or is there other cases where this function would be used for other sources than the dso?

I think it should also be created for fuel source sinks, eg. diesel or water, which currently is not working (issue #186). I would therefore not rename it.

Please run following test data to check whether or not the peak demand pricing function still works after you change the code (this one uses monthly peak pricing):
inputs.zip

actually these inputs did not work for me. But the error has nothing to do with my changes as far as I understand. I get an error in this line.
I could look where it comes from if no one else is working on that (apparently the dictionary "dict_values["energyProviders"][dso]" is not created correctly. But that would be another issue.

Let me check this today, I am really surprised it does not work.

@smartie2076
Copy link
Collaborator

Ah, some csv editor misformatted the line with the peak demand pricing.

 "peak_demand_pricing_period,""times per year (1,2,3,4,6,12)"",12"

instead of

peak_demand_pricing_period,"times per year (1,2,3,4,6,12)",12

Here are the updated files:
inputs_updated.zip

@Piranias
Copy link
Collaborator Author

Piranias commented Apr 8, 2020

Ah, some csv editor misformatted the line with the peak demand pricing.

 "peak_demand_pricing_period,""times per year (1,2,3,4,6,12)"",12"

instead of

peak_demand_pricing_period,"times per year (1,2,3,4,6,12)",12

Here are the updated files:
inputs_updated.zip

ah OK!
great, it works perfectly. So ready to review if you have time ;)

@SabineHaas SabineHaas mentioned this pull request Apr 9, 2020
7 tasks
Copy link
Collaborator

@smartie2076 smartie2076 left a comment

Choose a reason for hiding this comment

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

Wouldnt it also be possible to define a function in C0 that checks/adds the maximumCapacity info instead of defining it multiple times (with, I assume, the same code bits) in energyProduction, energyConversion and energyStorage?

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
src/C0_data_processing.py Outdated Show resolved Hide resolved
src/C0_data_processing.py Outdated Show resolved Hide resolved
src/C0_data_processing.py Outdated Show resolved Hide resolved
@@ -722,6 +810,9 @@ def define_source(dict_values, asset_name, price, output_bus, timeseries, **kwar
else:
source.update({"optimizeCap": {"value": False, "unit": "bool"}})

# add the parameter "maximumCap" to DSO source
source.update({"maximumCap": {"value": None, "unit": "kWp"}})
Copy link
Collaborator

Choose a reason for hiding this comment

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

not correct unit

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

what's the correct unit here? there is no row "unit" in energyProviders, so I have to write the unit hardcoded in this cas

@smartie2076
Copy link
Collaborator

smartie2076 commented Apr 15, 2020

Please add a description of the maximumCap parameter that you added in the readthedocs parameter description, or if more text is needed in a new custom_constraints.rst page/add it in the simulating_with_the_mvs.rst fild that is in the dev branch.

Piranias and others added 4 commits April 16, 2020 10:19
Co-Authored-By: smartie2076 <44204527+smartie2076@users.noreply.github.com>
Co-Authored-By: smartie2076 <44204527+smartie2076@users.noreply.github.com>
Co-Authored-By: smartie2076 <44204527+smartie2076@users.noreply.github.com>
@Piranias
Copy link
Collaborator Author

Please add a description of the maximumCap parameter that you added in the readthedocs parameter description, or if more text is needed in a new custom_constraints.rst page/add it in the simulating_with_the_mvs.rst fild that is in the dev branch.

I added the parameter here readthedocs parameter description
I didn't find a reasonable spot where to add it in the simulating_with_the_mvs.rst file

@Piranias Piranias merged commit 692bae1 into dev Apr 16, 2020
@Piranias Piranias deleted the feature/maxCapII branch April 16, 2020 09:34
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.

None yet

2 participants