-
Notifications
You must be signed in to change notification settings - Fork 10
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
Pandas wrapper #483
Pandas wrapper #483
Conversation
-adjusted strategies for new format -fixed tests
-rename FastDateTimeSeries to FastSeries -adjust bidding strategies to use zip for better readability and speed
-fix dmas strategies
-fix flexable storage startegy -adjust RL strategies to work with new format -adjust steelplant code to use new index -fix several tests
-fix extended strategy
@maurerle all tests pass except for one in unit_operator def test_get_actual_dispatch. I don't understand why it doesn't work, could you please take a look into it? |
the format of the unit_dfs output did change, so we need to adapt the test to it as it does not use pandas dataframe anymore
Somehow the other tests for RL are not yet working @nick-harder |
-fix RL tests by passing learning bidding strategies during initialization
@maurerle thanks for the fix, I have also fixed the RL tests now |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #483 +/- ##
==========================================
+ Coverage 76.40% 76.61% +0.20%
==========================================
Files 49 50 +1
Lines 6295 6727 +432
==========================================
+ Hits 4810 5154 +344
- Misses 1485 1573 +88
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
@kim-mskw @maurerle I have tested all non learning examples and everythings works fine, all data is also being properly written to the database and other outputs. I have also replaced and removed pandas from the code as much as possible and renamed the typing hints and other things. Please take another look at the commit |
84b7c80
to
a0d39b3
Compare
a0d39b3
to
259043b
Compare
@maurerle @kim-mskw @Manish-Khanra I have been testing now with the largest example we have for 2019 with two EOM and CRM markets, and the speed up is crazy, I didn't expect that. The difference is around 4.2 times. We should really all put a bit more effort and merge this as soon as we can after proper testing, this performance increase is even more what we expected... |
@kim-mskw I have tested the RL examples we have. Some things had to be fixed byt everything is working and the learning in small 1 and 2 is also like 2 to 3 times faster |
…for each different example
Note that this implementation also has This is fixed here as well (by using numpy and not pandas anymore) and commenting out does not have a noticable influence on the performance now. So we have a better fix for #355 and #357 now and economic results are again available in the standard calculation |
Of course :) on it! I have one minor comment left. |
-save dfs when size limit is reached
Pull Request
Related Issue
Closes #321
Description
As suggested by @maurerle this improves performance of all simulations by a factor of 2 up to 4, by replacing pandas actions with numpy where possible. For this, we are using a wrapper object FastIndex and FastSeries which wraps a numpy array so that we can access it using typical datetime accessors.
The speed up for small simulations is 2x and for large simulations 3x.
Changes Proposed
Testing
Most of the tests pass and simulations work, but a more extensive testing is required to test full functionality.
Checklist
Please check all applicable items:
doc
folder updates)pyproject.toml
doc/release_notes.rst
of the upcoming release is includedAdditional Notes (if applicable)
[Any additional information, concerns, or areas you want reviewers to focus on]
Screenshots (if applicable)
[Add screenshots to demonstrate visual changes]