Skip to content

Feature/coupled maximumCap#848

Merged
ciaradunks merged 19 commits intodevfrom
feature/coupled_maximum_installed_capacities
Apr 21, 2021
Merged

Feature/coupled maximumCap#848
ciaradunks merged 19 commits intodevfrom
feature/coupled_maximum_installed_capacities

Conversation

@ciaradunks
Copy link
Copy Markdown
Contributor

@ciaradunks ciaradunks commented Apr 6, 2021

Fix #Issue

Changes proposed in this pull request:

  • Introduce new parameter MaximumAddCap (where MaximumAddCap=MaximumCap-InstalledCap) in C0.process_maximum_cap_constraint
  • Deactivate optimization when MaximumCap<=InstalledCap in C0.process_maximum_cap_constraint
  • Fix MaximumCap definition in RTD and include definition for MaximumAddCap
  • Replace usage of MaximumCap in D1 with MaximumAddCap calculated in C0

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

  • Use in-line comments to explain your code
  • ❌ Write docstrings to your code (example docstring)
  • For new functionalities: Explain in readthedocs
  • ❌ Write test(s) for your new patch of code (pytests, assertion debug messages)
  • Update the CHANGELOG.md
  • Apply black (black . --exclude docs/)
  • Check if benchmark tests pass locally (EXECUTE_TESTS_ON=master pytest)

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

@ciaradunks ciaradunks changed the title Modify C0.process_maximum_cap_constraint Feature/coupled maximumCap Apr 6, 2021
@ciaradunks ciaradunks marked this pull request as draft April 6, 2021 08:33
@ciaradunks ciaradunks force-pushed the feature/coupled_maximum_installed_capacities branch 4 times, most recently from 94ccd1d to 9c5d26c Compare April 12, 2021 13:46
@ciaradunks
Copy link
Copy Markdown
Contributor Author

@smartie2076 after making the suggested changes and running the tests, KeyError: 'maximumAddCap' is returned from D1.transformer_constant_efficiency_optimize(), for instance. Do you have an idea of why this is happening?

@smartie2076
Copy link
Copy Markdown
Collaborator

Yes, I do! It is the same thing that was the case in the other PR: The input files for the D0 and D1 tests are json files: tests/test_data/inputs_for_D0/mvs_config.json and tests/test_data/inputs_for_D1/mvs_config.json. You need to add the new parameter there as well... :/

@ciaradunks ciaradunks force-pushed the feature/coupled_maximum_installed_capacities branch from 7523af5 to 3262688 Compare April 13, 2021 07:36
@ciaradunks ciaradunks marked this pull request as ready for review April 13, 2021 07:58
@ciaradunks ciaradunks requested a review from smartie2076 April 13, 2021 07:59
Copy link
Copy Markdown
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.

I didnt get that far yet with my review, sorry. Will continue later.

Copy link
Copy Markdown
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.

Nice, almost done! There is this pending issue about which error types we should use for the MVS, I hope @Bachibouzouk can enlighten us ;)

Do you also see the benefits of also having an MAXIMUM_ADD_CAP_NORMALIZED? It would only be used in energyProduction assets...

@ciaradunks ciaradunks force-pushed the feature/coupled_maximum_installed_capacities branch 2 times, most recently from 99b3d04 to 92f79c7 Compare April 19, 2021 08:21
@ciaradunks ciaradunks force-pushed the feature/coupled_maximum_installed_capacities branch from 29895eb to 3f06a5a Compare April 21, 2021 08:16
@ciaradunks ciaradunks force-pushed the feature/coupled_maximum_installed_capacities branch from 3f06a5a to 8890878 Compare April 21, 2021 09:04
@ciaradunks ciaradunks merged commit dc78992 into dev Apr 21, 2021
@ciaradunks ciaradunks deleted the feature/coupled_maximum_installed_capacities branch April 21, 2021 09:09
@smartie2076 smartie2076 mentioned this pull request May 31, 2021
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.

3 participants