Skip to content

Fix/feedin tariff alternative#685

Merged
SabineHaas merged 35 commits intodevfrom
fix/feedin_tariff_alternative
Dec 8, 2020
Merged

Fix/feedin tariff alternative#685
SabineHaas merged 35 commits intodevfrom
fix/feedin_tariff_alternative

Conversation

@smartie2076
Copy link
Copy Markdown
Collaborator

@smartie2076 smartie2076 commented Dec 4, 2020 •
edited
Loading

Fix #610

Changes proposed in this pull request:

  • set feed-in tariff to negative when executing C0.define_sink in C0.define_dso_sinks_and_sources
  • ❌ No adaptation of C1.check_feedin_tariff() necessary
  • added benchmark test for feed-in tariff
  • make sure that feedin revenue is indeed substracted from the operational costs
  • ❌ give a hint in readthedocs about feed-in tariff sign --> looks fine in rtd: "Price received for feeding electricity into the grid." implies that it's positive.

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)

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.

@smartie2076 smartie2076 force-pushed the fix/feedin_tariff_alternative branch from 5b154b1 to 198593f Compare December 4, 2020 21:45
@smartie2076 smartie2076 force-pushed the fix/feedin_tariff_alternative branch from 198593f to 49f1c43 Compare December 4, 2020 23:05
@smartie2076
Copy link
Copy Markdown
Collaborator Author

smartie2076 commented Dec 4, 2020

@SabineHaas this PR can now be reviewed. I also added a lot of pytests, but if you review commit by commit everything should be clear.

@smartie2076
Copy link
Copy Markdown
Collaborator Author

smartie2076 commented Dec 4, 2020

I have not checked the investments and if the costs add up. Especially for the pie charts, the revenues from feedin might result in weird stuff. I would say though that we do that in a new PR, eg. when merging #613?

@SabineHaas
Copy link
Copy Markdown
Contributor

SabineHaas commented Dec 7, 2020

Thank you for reworking @smartie2076. Unfortunately I still get the error mentioned in #670. Apart from that, two more benchmark tests fail now when I run EXECUTE_TESTS_ON=master pytest locally, see error log:
pytest_master_log.txt

Could you please run the test locally again and check?

@smartie2076
Copy link
Copy Markdown
Collaborator Author

smartie2076 commented Dec 7, 2020

Thank you for reworking @smartie2076. Unfortunately I still get the error mentioned in #670. Apart from that, two more benchmark tests fail now when I run EXECUTE_TESTS_ON=master pytest locally, see error log:
pytest_master_log.txt

Could you please run the test locally again and check?

So weird, when I ran them Friday they seemed to pass... will check what this is about.

@SabineHaas
Copy link
Copy Markdown
Contributor

SabineHaas commented Dec 7, 2020

Thank you for reworking @smartie2076. Unfortunately I still get the error mentioned in #670. Apart from that, two more benchmark tests fail now when I run EXECUTE_TESTS_ON=master pytest locally, see error log:
pytest_master_log.txt
Could you please run the test locally again and check?

So weird, when I ran them Friday they seemed to pass... will check what this is about.

Thank you so much!

@smartie2076 smartie2076 mentioned this pull request Dec 7, 2020
Merged
9 tasks
@smartie2076
Copy link
Copy Markdown
Collaborator Author

smartie2076 commented Dec 7, 2020

@SabineHaas fixed the pytests. Go commit by commit, because somehow the files that were changed in another PR actually appear in this one now...

@smartie2076
Copy link
Copy Markdown
Collaborator Author

smartie2076 commented Dec 8, 2020 •
edited
Loading

@SabineHaas ready to be merged!

If there is still something wrong, I am really at a loss here, as my pytests run trough:=========================================================== 319 passed, 2 skipped, 14 warnings in 529.66s (0:08:49) ============================================================

@SabineHaas
Copy link
Copy Markdown
Contributor

SabineHaas commented Dec 8, 2020

@SabineHaas ready to be merged!

If there is still something wrong, I am really at a loss here, as my pytests run trough:=========================================================== 319 passed, 2 skipped, 14 warnings in 529.66s (0:08:49) ============================================================

:D
I've checked it, it's fine 👍

@SabineHaas SabineHaas merged commit a7d7818 into dev Dec 8, 2020
@SabineHaas SabineHaas deleted the fix/feedin_tariff_alternative branch December 8, 2020 14:29
@SabineHaas SabineHaas mentioned this pull request Dec 8, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@SabineHaas SabineHaas SabineHaas approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Feed-in tariff in 'energyProvider.csv' should be negative

2 participants

@smartie2076 @SabineHaas